[CLOSED] Added 'All' button to FindReplace functionality.

[core-ai-bot]

Issue by zaggino
Wednesday Aug 07, 2013 at 04:39 GMT
Originally opened as adobe/brackets#4686

---

I've been missing 'All' button when replacing too many occurrences of a string in file. I can add tests if this gets considered for merge.

---

zaggino included the following code: https://github.com/adobe/brackets/pull/4686/commits

[core-ai-bot]

Comment by TomMalbran
Wednesday Aug 07, 2013 at 06:01 GMT

---

Nice feature!

I was thinking that after doing a replace All, we could show what was replaced, in similar format to find in files. So you could go through every replace and check if the result is what you expected and easily undo a replace if it wasn't.

Any thoughts?

[core-ai-bot]

Comment by zaggino
Wednesday Aug 07, 2013 at 06:05 GMT

---

Yes, it could certainly be useful to see what was replaced. This was just a quick idea because I was stuck with pressing 'Yes' for 50 times in one file :)

Are you thinking of doing it for current file, or global style command "Replace in Files" (Ctrl + Shift + H) ?

[core-ai-bot]

Comment by TomMalbran
Wednesday Aug 07, 2013 at 06:24 GMT

---

Hehe. I did that before, and even worse I ended clicking so fast that clicked in between the closing and opening of the dialog, making it disappear...

It is a nice feature to add, and we really need it. But I think it should be well implemented too.

I was actually thinking it just for the current file, since we don't have a Replace in Files yet. Anyway, it should be useful for both commands.

[core-ai-bot]

Comment by zaggino
Wednesday Aug 07, 2013 at 06:27 GMT

---

Ok, I'll give it a try in spare time, maybe I'll come up with something usable.

[core-ai-bot]

Comment by TomMalbran
Wednesday Aug 07, 2013 at 06:40 GMT

---

Or wait to see what others have to say about it. Maybe we don't need it yet or don't need it for the current Replace command. But if you still want to try it, go ahead.

I was thinking of saving for each replacement, the line, the string, and the column position and length of the new string. Then use this information to fill a bottom panel. Similar to how Find in Files works.

[core-ai-bot]

Comment by zaggino
Thursday Aug 08, 2013 at 06:04 GMT

---

@TomMalbran can you look at the last commit ? I can't figure out why AppInit.htmlReady gets launched when no #status-bar element is available, which causes PanelManager.createBottomPanel to attach panel to nowhere.

There is no check in PanelManager.createBottomPanel to see if #status-bar exists.

    function createBottomPanel(id, $panel, minSize) {
        $panel.insertBefore("#status-bar");
        $panel.hide();
        updateResizeLimits();  // initialize panel's max size

        return new Panel($panel, minSize);
    }

[core-ai-bot]

Comment by TomMalbran
Thursday Aug 08, 2013 at 06:15 GMT

---

The status bar is also created on htmlReady. Which means that the html ready for this file could be called before the html ready of the Status bar code.

To solve this, the status bar div (empty) should be included in the main view template, and when creating the status bar, it should just include the content inside the status bar div.

[core-ai-bot]

Comment by julianasuh
Thursday Aug 08, 2013 at 17:28 GMT

---

Hi@TomMalbran, do you want to review this pull request?

[core-ai-bot]

Comment by julianasuh
Thursday Aug 08, 2013 at 17:28 GMT

---

@larz0 to review UI

[core-ai-bot]

Comment by TomMalbran
Thursday Aug 08, 2013 at 21:12 GMT

---

@julianasuh Sure. I can.

[core-ai-bot]

Comment by larz0
Thursday Aug 08, 2013 at 22:11 GMT

---

@zaggino UI looks good but nothing happens when I click "All". Not sure if it's just me.

[core-ai-bot]

Comment by zaggino
Thursday Aug 08, 2013 at 22:35 GMT

---

@larz0 I haven't fully submitted it yet, working on it right now. I'll let you know today when I have first reviewable version.

[core-ai-bot]

Comment by larz0
Thursday Aug 08, 2013 at 23:55 GMT

---

@zaggino ahh cool. Thanks for this BTW.

[core-ai-bot]

Comment by zaggino
Thursday Aug 08, 2013 at 23:58 GMT

---

Ok, first go at UI, right now it looks like this, probably you can get an idea where it is heading from it.

[core-ai-bot]

Comment by zaggino
Thursday Aug 08, 2013 at 23:59 GMT

---

Check All - None should probably be replaced by checkbox left of "Search for ..."