-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Extend is_case_difference
to handle digit-letter confusables
#144691
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of nitpicks.
1295c6a
to
10af3e1
Compare
This comment has been minimized.
This comment has been minimized.
#[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
pub enum ConfusionType { | ||
/// No confusion detected | ||
None, | ||
/// Only case differences (e.g., "hello" vs "Hello") | ||
Case, | ||
/// Only digit-letter confusion (e.g., "0" vs "O", "1" vs "l") | ||
DigitLetter, | ||
/// Both case and digit-letter confusion | ||
Both, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I define a enum to handle different cases.
help: write it in the correct case (notice the capitalization difference) | ||
help: write it in the correct case (notice the capitalization) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remove difference
to keep consistency.
|
||
let ascii_confusables = &['c', 'f', 'i', 'k', 'o', 's', 'u', 'v', 'w', 'x', 'y', 'z']; | ||
|
||
let digit_letter_confusables = [('0', 'O'), ('1', 'l'), ('5', 'S'), ('8', 'B'), ('9', 'g')]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep simple and use reverse
Signed-off-by: xizheyin <[email protected]>
10af3e1
to
7b667e7
Compare
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
@bors r+ |
Rollup of 7 pull requests Successful merges: - #143849 (rustdoc: never link to unnamable items) - #144683 (Simplify library dependencies on `compiler-builtins`) - #144691 (Extend `is_case_difference` to handle digit-letter confusables) - #144700 (rustdoc-json: Move `#[macro_export]` from `Other` to it's own variant) - #144751 (Add correct dynamic_lib_extension for aix) - #144757 (Ping Muscraft when emitter change) - #144759 (triagebot: Label `compiler-builtins` T-libs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #144691 - xizheyin:suggest-confuse, r=estebank Extend `is_case_difference` to handle digit-letter confusables This PR extends `is_case_difference` to handle digit-letter confusables Add support for detecting 0/O, 1/l, 5/S, 8/B, 9/g confusables in error suggestions. r? `@estebank`
|
||
let ascii_confusables = &['c', 'f', 'i', 'k', 'o', 's', 'u', 'v', 'w', 'x', 'y', 'z']; | ||
|
||
let digit_letter_confusables = [('0', 'O'), ('1', 'l'), ('5', 'S'), ('8', 'B'), ('9', 'g')]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about 1 and capital i?
And lowercase L and capital i?
This PR extends
is_case_difference
to handle digit-letter confusablesAdd support for detecting 0/O, 1/l, 5/S, 8/B, 9/g confusables in error suggestions.
r? @estebank