Adds option for strongly preferred prefix and single segment search. #3588
Conversation
These options make stringMatch produce useful and predictable results in cases that are not quite the same as file searching (such as code hinting).
ghost
assigned 
Adding this option to string matching provides a fix for #3534 by putting the matches at the same score and using the scope as a tie breaker.
|
@peterflynn I'd like to get this in for sprint 24 as it takes care of #3534. |
|
Reviewing |
| * @return {{ranges:Array.<{text:string, matched:boolean, includesLastSegment:boolean}>, matchGoodness:int, scoreDebug: Object}} matched ranges and score | ||
| */ | ||
| function stringMatch(str, query, special) { | ||
| function stringMatch(str, query, options, special) { |
There was a problem hiding this comment.
This is public API, so it would be better to add the new arg at the end for backwards compatibility.
There was a problem hiding this comment.
You're probably right, though I made this choice because the special parameter is just a speed optimization used by StringMatcher and is not likely used elsewhere. options, on the other hand, is likely to be used.
I could keep the new signature and deprecate the old by looking for the presence of special.specials. What do you think?
There was a problem hiding this comment.
Good point -- in light of that, it's fine by me as-is
|
Done reviewing body code -- will review unit test part after I grab lunch |
| expect(result.stringRanges[0]).toEqual( | ||
| { text: "str", matched: true, includesLastSegment: true } | ||
| ); | ||
| }); |
There was a problem hiding this comment.
What about a test for singleSegmentSearch used in isolation? Although core code doesn't do that (yet), it is exposed in the API...
|
@dangoor Done reviewing |
...and some other small changes based on review feedback
|
I'm going to remove the sprint 24 flag from this. I'd like for this change to get in properly, and if it's sprint 25 that's fine. |
|
Looking pretty good. I think the only things remaining are:
|
|
Oh also -- do you want to include a change in the JavaScriptCodeHints extension to use this flag? |
|
Looks like this could fix #3643. |
|
Nominated pull request for Sprint 25 -- added to card in Trello |
Conflicts: src/extensions/default/JavaScriptCodeHints/main.js
…aults to false (which is a change in behavior for quick navigate that should actually be fine for those uses). Also augmented the test cases based on review feedback.
|
@peterflynn I've renamed singleSegmentSearch to segmentedSearch (and flipped the meaning with the default being "false", so now Quick Open for files explicitly sets this). JS Code Hints does set the prefixPrefixMatches option. I think I've addressed all review comments except for allowing the match options to be passed in via pluginDef. I can't take care of that right this second, but I will do so either this evening or tomorrow morning. |
|
@dangoor Changes look perfect so far! Let me know when the last bit is in... |
|
OK, I just pushed the last part. Quick Open plugins can now include |
|
Awesome! Merging... I'll also add a placeholder to the release notes for the API enhancements. |
Add QuickOpen / StringMatch options for strongly preferred prefix and unsegmented search weighting.
These options make stringMatch produce useful and predictable results in cases that are not
quite the same as file searching (such as code hinting).