[CLOSED] Prepopulate replace input with current editor selection

[core-ai-bot]

Issue by jbalsas
Friday Oct 26, 2012 at 08:51 GMT
Originally opened as adobe/brackets#1964

---

Prepopulates the replace input with the current editor selection.

This pull request replaces adobe/brackets#1962 following@peterflynn suggestions.

---

jbalsas included the following code: https://github.com/adobe/brackets/pull/1964/commits

[core-ai-bot]

Comment by peterflynn
Friday Oct 26, 2012 at 19:52 GMT

---

This looks good, but one thing I noticed is that the replace begins on the next occurrence after the text you initially selected. It loops back around eventually, but the text you had selected will always be last to get replaced. This bug already existed, of course, but your change makes it easier to hit since there's now an incentive to have some text selected.

I think you can fix this by changing line 225 from cm.getCursor() to cm.getCursor(true) (i.e., always begin at the start of the selection, instead of whichever end the cursor was left at). Can you test this out and add it to your pull request if it works?

[core-ai-bot]

Comment by jbalsas
Friday Oct 26, 2012 at 22:09 GMT

---

Yes, I noticed that as well, but as it was already happening in master and is stated in the comments at the top I thought about trying to solve it separately...

Something similar happens if you have the cursor in the middle of a word (no selection) which looks strange to me. However, I've been testing it and Sublime2 does exactly the same.

@peterflynn I've tested your solution and setting cm.getCursor(true) does fix this indeed. Do you also want me to take out or update the Replace works a little oddly [...] comment at the top?

[core-ai-bot]

Comment by jbalsas
Sunday Oct 28, 2012 at 09:38 GMT

---

@peterflynn I've already pushed the changes.

After reading it again, I think the comment I was quoting refers to the find-next in general, which still works like that. This could also be fixed by setting cm.getCursor(true) in line 151 inside doSearch. I could push it here as well, but maybe it makes more sense to create a differente pull request. What do you think?

[core-ai-bot]

Comment by peterflynn
Monday Oct 29, 2012 at 23:49 GMT

---

@jbalsas: I actually think that comment is no longer correct even before your change, so we should probably just remove it. It originates from a very old version of the file, where it looks like Find and Replace were much more entangled. In more recent versions of the code, findNext() never does any replacement.

But we can do that cleanup later -- I'm happy to merge this pull request now.