Highlight substrings correctly and initial exclusion support with -#131215
Highlight substrings correctly and initial exclusion support with -#131215TylerLeonhardt wants to merge 2 commits intomainfrom
-#131215Conversation
|
|
||
| // Prefer label matches if told so or we have no description | ||
| if (preferLabelMatches || !description) { | ||
| if ((preferLabelMatches && !query.excludeQuery) || !description) { |
There was a problem hiding this comment.
this is a bit tricky... but the idea is that exclude queries have to happen on the entire path.
| /* | ||
| * If a query is wrapped in quotes, the user does not want to | ||
| * use fuzzy search for this query. | ||
| */ |
There was a problem hiding this comment.
| /* | |
| * If a query is wrapped in quotes, the user does not want to | |
| * use fuzzy search for this query. | |
| */ | |
| /* | |
| * If a query starts with a dash and isn't just a dash, the user does not want this | |
| * query to appear in results. | |
| */ |
|
I am wondering how the highlighting is not working properly when using quotes for forcing contiguous matches, isn't that the same that is used for editor history matches, so I would expect we had a bug there already? I suggest to split the PR up into fixing the highlighting and implementing the support for |
|
Oh yeah, I can confirm the highlighting issue, good catch. Reproduces in stable too with entries from editor history:
|
bpasero
left a comment
There was a problem hiding this comment.
High level feedback on the matching fix:
- rename
fuzzy: booleantoallowNonContiguousMatchesto make the intent clear (the current wording is confusing) - not a big fan of bypassing the scorer matrix for when an exact match is searched, can we not use it but assign a score of 0 within the matrix when we are expecting exact matches and the current match is not contiguous? that way we are not breaking any ranking behaviour we have today already
|
Ok I've moved the "quote" support to #131292 |
|
I am not sure whether we want to push forward on support for
As such I would think the code changes are relatively small. |
|
I agree, @bpasero. I think it'd be easy to add. Before I go and add |
I’ve been playing around with a few ways to hone in on the files you’re looking for in the cmd+P file search. Taking inspiration from Google & GitHub here’s what quoting and exclusion might look like:
The results:
(exclude query pieces can only be used if there’s another piece)
(“pieces” of a query are separated by spaces so asdf bsdf has 2 pieces)
Initially I felt like the exclusion query pieces weren't needed but then I used it more and found it really nice tacking on additional exclusions.
2 commits initially:
"26"the highlight is 2021-9-26 and not 2021-9-26 (if that's hard to read, the 2nd one has highlights on the first 2 instead of the 2 next to the 6)-fooexclusion query partsI would recommend looking at the first commit first and then the 2nd.
I haven't fixed up the tests yet but I was hoping to get some feedback from @bpasero before I do that to make sure I'm on the right track generally speaking.
This PR fixes #128923