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.

Sign up for GitHub

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

Jump to bottom

Make levenshtein distance case insensitive. #10224

Merged
merged 2 commits into from
Dec 24, 2021
Merged

Make levenshtein distance case insensitive. #10224

merged 2 commits into from
Dec 24, 2021

Conversation

MidasLamb
Copy link
Contributor

When typing in a single character shortcut as a capital, it always
returns b as the suggestion as every one-letter abbreviation
is a lev distance 1 away from the capitalized one.
By making the levenshtein distance case insensitive, the case-mismatched
one-letter abbriviation (e.g. C to c) will be suggested, rather
than b

@MidasLamb
Make levenshtein distance case insensitive.
4252879 
When typing in a single character shortcut as a capital, it always
returns `b` as the suggestion as every one-letter abbreviation
is a lev distance 1 away from the capitalized one.
By making the levenshtein distance case insensitive, the case-mismatched
one-letter abbriviation (e.g. `C` to `c`) will be suggested, rather
than `b`
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 21, 2021
@alexcrichton
Copy link
Member

Thanks for the PR! Do you know if there's precedent for doing something like this? Do other levenshtein-using "did you mean" systems also do case-insensitive compares?

@MidasLamb
Copy link
Contributor Author

I couldn't find any precedent for this, I just noticed that I accidentally had caps lock on and I found it weird that it would suggest b when I typed C.
Conceptually I find that C to c should have less distance than C to b, but for Levenshtein a change is a change, and there is no metric for "how far" the letters are apart that it needs to change into.
Potentially we could change the scoring such that changing capitalization has a cost (i.e. 0.5) as a special case, but that would be a bigger and more complex change (this basically does the same thing except with a cost of 0 for change of capitalization).

@alexcrichton
Copy link
Member

Ok well at least seems reasonable to me. Can you add comments to the .to_lowercase() calls though as to why they're being made?

@MidasLamb
Copy link
Contributor Author

Will do!

@joshtriplett
Copy link
Member

Looks good to me!

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 24, 2021

📌 Commit f0992e3 has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 24, 2021
@bors
Copy link
Collaborator

bors commented Dec 24, 2021

⌛ Testing commit f0992e3 with merge 1f12b88...

@bors
Copy link
Collaborator

bors commented Dec 24, 2021

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing 1f12b88 to master...

@bors bors merged commit 1f12b88 into rust-lang:master Dec 24, 2021
@ehuss ehuss mentioned this pull request Jan 5, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 5, 2022
@bors
Auto merge of rust-lang#92567 - ehuss:update-cargo, r=ehuss
26c9b00 
Update cargo

10 commits in fcef61230c3b6213b6b0d233a36ba4ebd1649ec3..358e79fe56fe374649275ca7aebaafd57ade0e8d
2021-12-17 02:30:38 +0000 to 2022-01-04 18:39:45 +0000
- Make rmeta_required no longer depend on whether timing is enabled (rust-lang/cargo#10254)
- The first version of pull request template (rust-lang/cargo#10218)
- Stabilize the `strip` profile option, now that rustc has stable `-C strip` (rust-lang/cargo#10088)
- Update docs for windows ssh-agent. (rust-lang/cargo#10248)
- Fix typo: substract -> subtract (rust-lang/cargo#10244)
- timings: Fix tick mark alignment (rust-lang/cargo#10239)
- Remove unused lifetimes (rust-lang/cargo#10238)
- Make levenshtein distance case insensitive. (rust-lang/cargo#10224)
- [docs] Adds basic CI yaml for GitHub Actions (rust-lang/cargo#10212)
- Add function for parsing already-read manifest (rust-lang/cargo#10209)
@ehuss ehuss added this to the 1.59.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.
6 participants
@MidasLamb @rust-highfive @alexcrichton @joshtriplett @bors @ehuss