Only use first line of primary selection when populating FindInFiles bar

[marcelgerber]

For #8211

[JeffryBooher]

I'm getting an RTE:

Uncaught TypeError: Object #<Editor> has no method 'getSelectionText'

[JeffryBooher]

Did you forget to add changes to editor.js maybe?

[JeffryBooher]

This should have some unit tests : some unit tests for Editor.getSelectionText() and some integration tests for Find to ensure only 1 line shows in the modal bar.

[marcelgerber]

@JeffryBooher The function was actually called getSelectedText().
I'll add unit tests soon.

[marcelgerber]

@JeffryBooher Unit tests added.

[JeffryBooher]

The Selection in the current editor isn't updated to reflect the new initial string like it is when searching in a single file. In FindReplace.js we make a call to handleQueryChange(editor, state, true); that we also need to do when searching across files due to the changing of the initial string in FindInFiilesUI.js

[peterflynn]

Could you hoist this into FindUtils so it's not duplicated between FindInFilesUI & FindReplace? Maybe a utility like FindUtils.getInitialQueryFromSelection()?

[peterflynn]

Also, the flow here could probably be cleaned up to something more like this:

var initialString = "";
if (_findBar && !_findBar.isClosed()) {
    initialString = ...;
} else if (currentEditor) {
    initialString = FindUtils.getQueryFromSelection(currentEditor);
}

[marcelgerber]

@peterflynn I tend to introduce a more generic function, like StringUtils.getFirstLine(). Is that ok with you?

[peterflynn]

I'm not sure there's that much of a wider use for it (do you see other copies of this pattern elsewhere in core yet?). But also, putting something in FindUitls means you can encapsulate the getSelection() call too, and there's only one place to update when we later change to support multi-line selections.

[redmunds]

@marcelgerber I haven't looked at the code, but the current selection should be used (not the first). Note that the current selection is the last selection added chronologically (nothing to do with order in file).
cc @JeffryBooher

[JeffryBooher]

I think it's fine to make "FindInFiles" work the way that "Find" works -- which is how this is written. @peterflynn suggested that we combine the two implementations into a single utility function (which I agree with) so we can change it there at a later date if it makes sense. Personally I think it's fine that it uses the first line in the selection that's it's the first thing the user will see so it will make sense to the user.

[peterflynn]

@redmunds That should be how this works already: it uses Editor.getSelectedText() / getSelection(), which do return the current selection. The "first" in the title of the PR is referring to how the text gets trimmed down within that single current selection -- if the current selection spans multiple lines, we truncate it to just the first line.

[redmunds]

@peterflynn That sounds right. I was confused by wording of title.

[JeffryBooher]

@peterflynn the way I understood @redmunds comment was that if I select starting on line 21 and select upwards through line 15 then whatever was on line 21 would be the line that was chosen. Currently we choose whatever is on line 15 and disregard the order in which the selections were added to the result.

[marcelgerber]

@JeffryBooher Re this comment: I don't think we should change the selection. In the single-file Find, we update the results on every query change, which is the only reason why the selection changes, but of course that isn't possible with mutliple files.

[peterflynn]

@JeffryBooher, re:

the way I understood @redmunds comment was that if I select starting on line 21 and select upwards through line 15 then whatever was on line 21 would be the line that was chosen

I'm pretty sure he was talking about multiple, separate selections (the "multiple cursors" feature). And this code already behaves correctly in that regard.

Re:

The Selection in the current editor isn't updated to reflect the new initial string like it is when searching in a single file.

I agree with @marcelgerber -- Find in Files is not an incremental search, so we don't update the selection in the current editor when the query is edited. Tweaking that behavior seems outside the scope of this pull request, in any event...

[marcelgerber]

@peterflynn Changes pushed.

[redmunds]

@JeffryBooher Yes, I was referring to multiple selections, not a multiline selection. For a multiline selection I would expect it to behave the same for both selection directions (in a LTR language like English, anyway).

[peterflynn]

Looks good to me! Thanks @marcelgerber