Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boost for first letter matches #5365

Merged
merged 20 commits into from
Dec 11, 2024
Merged

boost for first letter matches #5365

merged 20 commits into from
Dec 11, 2024

Conversation

willmcgugan
Copy link
Collaborator

Tweak to the command palette matcher to boost matches for the first word(s).

Consider the system commands. If I were to type "SS" I would get "Show Keys and Help Panel". Now I get "Save Screenshot" because I'm matching the first letters.

Knowing this makes it easier to get a precise match.

@@ -40,6 +40,10 @@ def __init__(
".*?".join(f"({escape(character)})" for character in query),
flags=0 if case_sensitive else IGNORECASE,
)
self._first_word_regex = compile(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a new Matcher created on each key press in the command palette?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah

@willmcgugan
Copy link
Collaborator Author

Actually, I've nerd sniped myself. We can do so much better with the matching, but regexes aren't going to be enough.

@willmcgugan willmcgugan marked this pull request as draft December 9, 2024 11:16
@willmcgugan
Copy link
Collaborator Author

Putting this on hold. I'm going to show great strength of character and not work on this, because there are higher priorities right now.

@darrenburns
Copy link
Member

@willmcgugan Do you want to merge this to get the improvement in or stick with what we have?

@willmcgugan
Copy link
Collaborator Author

Going to hold off merging this. It's not quite a 100% improvement.

@willmcgugan willmcgugan marked this pull request as ready for review December 10, 2024 11:35
@willmcgugan
Copy link
Collaborator Author

@darrenburns slept on it, and came up with this.

You may want to check it against Posting.

@darrenburns
Copy link
Member

Cool - I'll try it against Posting soon.

@darrenburns
Copy link
Member

Minor but these look the wrong way around:

image

Should the first s in response not match here, and mean that these results should be in the opposite order?

Also I'm not sure about underlining the spaces.

@willmcgugan
Copy link
Collaborator Author

The current highlighting is the best score. The way it is highlighted includes the s of section which gets a points boost because it is the first letter. If you were to highlight the s in response you wouldn't get that.

You can match spaces, but I agree not highlighting them would be better.

@willmcgugan
Copy link
Collaborator Author

Wait, I see what you mean. It could match res and sec.

I may be able to tweak the heuristic. ATM it gives a boost to single words. It could boost based on the least number of groups.

@willmcgugan
Copy link
Collaborator Author

@darrenburns tweaked the heuristic so it favors matches with fewer groups.

@darrenburns
Copy link
Member

I'm not sure that was the problem as in the original example wasn't there 4 matching groups in both results?

It still seems like the first s in response is being skipped here, and the results aren't what I'd expect.

image

@willmcgugan
Copy link
Collaborator Author

My estimate for a reasonable maximum on the number of loops was too low.

Seems to work well with your example now. "exp res" puts response at the top.

@willmcgugan willmcgugan merged commit 268971e into main Dec 11, 2024
23 checks passed
@willmcgugan willmcgugan deleted the cp-first-word-boost branch December 11, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants