Keep query and replace text when switching between Replace and Replace In Files #9601
Conversation
marcelgerber
changed the title
| } else { | ||
| initialQuery = FindUtils.getInitialQueryFromSelection(editor); | ||
| var openedFindbars = FindBar._bars && FindBar._bars.filter(function (findBar) { |
There was a problem hiding this comment.
findBar is already used in module scope, so change it in this inner scope to something like bar to make code easier to read.
|
@marcelgerber This looks good. Just a couple minor comments. |
|
I wonder if we should put these lines into FindUtils to reduce code duplication? |
|
@redmunds Could you take another look? |
| query = currentFindBar.getQueryInfo().query; | ||
| replaceText = currentFindBar.getReplaceText(); | ||
| } else { | ||
| var openedFindbars = FindBar._bars && FindBar._bars.filter(function (bar) { |
There was a problem hiding this comment.
Since you're only interested in the first non-closed bar, you should use FindBar._bars.some() which quits the loop when first one is found (but filter() unnecessarily continues through entire Array).
There was a problem hiding this comment.
I used _.find() as Array.filter will only return a boolean value...
There was a problem hiding this comment.
Cool -- even better. With Array.some(), you have to create a var in outer scope to save item.
There was a problem hiding this comment.
... and that's why I didn't use it :D
|
@marcelgerber Nice refactoring! Just 1 more comment. |
|
@redmunds Time for the final review |
|
Looks good. Merging. |
Keep query and replace text when switching between Replace and Replace In Files
For #8531