Fix 2663: MultiRangeInlineEditor case #3819
Conversation
…earches when the user has typed at least two contiguous matching characters.
ghost
assigned 
|
to @peterflynn ... hopefully this is not as painful to review as other changes to the StringMatch algorithm. The change itself is fairly straightforward, and I think it's helpful, but I am open to the idea that this may not be the right sort of change. Interesting side note: I tried searching for "doc" in Sublime (3). Sublime puts DocumentCommandHandlers at the top (just barely). I honestly don't think that captures user intent as well as matching DocumentCommandHandlers. If I type "dch", I'm pretty clearly going for camelCase matches. That's not at all clear when typing "doc", though. |
|
@peterflynn The dangoor/prefer-prefix branch has landed, so the "Files Changed" is now nice and small. |
|
I'll see if I can handle this one since @peterflynn is out and it looks like he hasn't started on it yet. Nominating for sprint 27. |
|
|
||
| if (contiguousCount < 2) { | ||
| if (state === SPECIALS_MATCH) { | ||
| if (!findMatchingSpecial()) { |
There was a problem hiding this comment.
Now that there's an else, it might be clearer to reverse the sense of this if.
|
Initial review complete. |
|
@dangoor - just noticed this was still open, if you feel like working on it some more sometime soon :) See comments above. |
|
Closing this one. Will attack as part of other quick open fixes. |
Note that this branch is built on dangoor/prefer-prefix, which I suspect will land first.
This is a fix for #2663. The original "problem" there is that MultiRangeInlineEditor-test has a backtrack in it, whereas MultiRangeInlineEditor does not, so searching for "multir" results in a different sort of match between the two strings. (MultiRangeInlineEditor vs. MultiRangeInlineEditor-test).
Looking at this result, though, the "fix" (adjusting backtracking behavior to give the same result both times) would change both matches to be more like MultiRangeInlineEditor, when arguably they should be more like MultiRangeInlineEditor.
My fix should make the matching line up with scoring and user intent a bit better. If the user has already typed two characters in a row that match, the code now makes the assumption that the user may be trying to put a contiguous string together and tries that first. I think the results are generally better with this scheme. Note that it changed some pre-existing test cases (for the better, I think).
This change will also probably reduce some cases in which results appear to bounce around in the list as characters are typed.