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

Symbol picker displays confusing sort order on filter #4688

Closed
squarethecircle opened this issue Nov 10, 2022 · 5 comments · Fixed by #4698
Closed

Symbol picker displays confusing sort order on filter #4688

squarethecircle opened this issue Nov 10, 2022 · 5 comments · Fixed by #4698
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements

Comments

@squarethecircle
Copy link

squarethecircle commented Nov 10, 2022

Screen Shot 2022-11-09 at 5 31 32 PM
Fuzzy match scoring can definitely be a matter of taste, but in this case I would expect the exact match "TimeEntry" to be ranked higher than all the prefix matches. It's a bit painful to have to scroll down all the way to get to the one I want, as there's no way to narrow the match by typing more characters here.

@squarethecircle squarethecircle added the C-enhancement Category: Improvements label Nov 10, 2022
@kirawi
Copy link
Member

kirawi commented Nov 10, 2022

Are you using maser or the latest release? #3969 may have fixed this @pascalkuthe

@squarethecircle
Copy link
Author

Tried on latest release and master, the exact match is ranked lower on both.

@kirawi kirawi added the A-helix-term Area: Helix term improvements label Nov 10, 2022
@squarethecircle
Copy link
Author

fzf solves for this with the --tiebreak flag, which defaults to length (shorter result wins); perhaps that could make sense here as well?

@pascalkuthe
Copy link
Member

Thanks for tagging me @kirawi. #3969 does not change the sort order. I kept that PR minimal on purpose (the only thing I did there was treating space the same as fzf/skim).
Fzf and Skim are pretty complex with a ton of options and and I think we don't want all their features/match their behavior exactly so I only ported a feature I personally needed (that seemed uncontroversial).

For example for this exact case fzf and skim actually diverge.
fzf does highlight the shortest match (TimeEntry in this case) first but skimdoes not. Although both offer the--tiebreakflag to chance the behavior (although they work differently for both) I thinkfzf` generally gets these kinds of user-facing decisions right which leads some people to perceive its algorithm is superior (although the core fuzzy finder is actually somewhat better in skim I think).
The available options I found are:

  • byte length (fzf default, we should use char length but that might be slow)
  • byte position of match start (we should use char position but that might be slow)
  • byte position of match end (we should use char position but that might be slow)
  • index in unfiltered data (fzf default)

Furthermore both skim and fzf allow specifying multiple such tie break options (so if elements are still tied after the first tie breaker the second one applies).

I think hardcoding any of these options doesn't make much sense because different pickers might want different sorting orders (and users might even prefer different sorting orders or different sorting orders might even be nice for different languages). I think the current behavior can definitely be improved because we are doing an unstable sort right now (so the order is essentially random if the score is equal). I will open a PR where I implement the basic machinery to make this work (and be configurable in the picker) and then pick defaults for the various pickers. I am not sure what the best way to allow user configuration is. Potentially changing this at runtime might be nice (with some kind of indicator + a keybinding to change the tie breaker)

@pascalkuthe
Copy link
Member

fixed by #4698

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants