adobe/brackets — pull request #8137 by njx (2014-06-17 01:09:45+00:00)
Title: [replace-across-files] Code review fixes

This addresses most of the code review items from #7705 and #7809. Note that this is a pull into nj/replace-across-files - after this is merged, we'll need to put up another PR to actually merge it into master.

All the small cleanups are in c09eaef and 0f3cb18. In addition, I did two bigger cleanups:

* Moved more of the keyboard handling controller logic from FindInFilesUI into FindBar, making the events dispatched from FindBar much more clear.
* Made it so modifications to SearchModel results are done via APIs on SearchModel rather than manipulating `searchModel.results` directly, so that it can keep an accurate count of the number of matches without having to re-count every time results are added.

As part of the latter work I added a number of unit tests for the updating code (there was only one before), and in the process I found and fixed a bug in the line number update calculation (which has probably always been there).

There are a few code review issues that I didn't address, but I think we can land the feature without them:

* Clean up showError()/showNoResults() (combine them and/or make showNoResults() less confusing)
* Remove error-showing functionality from `FindReplace.parseQuery()`
* Don't pass replaceText to doSearchInScope() - have caller store it separately
* Move Replace tests to separate file
* Break out event handlers in SearchResultsView.addPanelListeners() (see Tom's comment in #7809)
* Move FindUtils.performReplacements() back into FindInFiles.doReplace()

I'll file a bug for these - I don't think they should impede shipping the feature this release.