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

Make MCFLY_FUZZY a tuneable int #203

Merged
merged 5 commits into from
Nov 4, 2021
Merged

Conversation

dmfay
Copy link
Contributor

@dmfay dmfay commented Oct 28, 2021

Experimenting based on discussion in #183

No extra weight:
fuzzy-factor-1

No elaboration needed, it's not great.

Shorter match weighted x2:
fuzzy-factor-2

Immediate improvement, with actual ssh commands at the top and most other sorting fairly reasonable. The ./target/relea[se/mcfly search] match is ahead of vi [sql.zsh], but I've definitely run the former several times in this directory (and the latter never).

Shorter match weighted x10:
fuzzy-factor-10

There's a point of diminishing returns here and 10 may be past it; ./target/relea[se/mcfly search] is pushed way down, and even higher values (x20, x50) don't look much different.

@cantino
Copy link
Owner

cantino commented Oct 28, 2021

What happens for folks who have it currently set to true or false? Make sure false maps to 0 and true to 1?

@dmfay
Copy link
Contributor Author

dmfay commented Oct 28, 2021

What happens for folks who have it currently set to true or false? Make sure false maps to 0 and true to 1?

Any string value resolves to 0, disabling fuzzy searching until an int factor >0 is supplied. It is a little disruptive, but I'm also thinking of people who might otherwise not realize there are improvements -- and, hopefully, getting a few more interested parties from the issue to help test and improve the weighting before this lands.

@cantino
Copy link
Owner

cantino commented Oct 29, 2021

I think it'd be better to be backward compatible and make 'true' map to '1' (or whatever numerical score you think has the best experience).

@cantino
Copy link
Owner

cantino commented Oct 29, 2021

Also, please run rustfmt. Thanks!

@dmfay
Copy link
Contributor Author

dmfay commented Oct 31, 2021

I had the idea of including match start position in the weighting, so earlier as well as shorter matches are preferred. That makes a big difference already, but a MCFLY_FUZZY of 2 still improves on 1.

if let Ok(fuzzy) = i16::from_str(&fuzzy) {
settings.fuzzy = fuzzy;
} else {
settings.fuzzy = 2;
Copy link
Owner

Choose a reason for hiding this comment

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

I think this needs to be something like (in pseudocode) if fuzzy.to_lowercase() != "false" { settings.fuzzy = 2; }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, you're right -- fixed.

@cantino cantino merged commit 2133871 into cantino:master Nov 4, 2021
@cantino
Copy link
Owner

cantino commented Nov 4, 2021

Thanks!

@cantino
Copy link
Owner

cantino commented Nov 6, 2021

Released in 0.5.10.

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