Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upImprove typo suggestion hueristics #26087
Conversation
rust-highfive
assigned
alexcrichton
Jun 8, 2015
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
rust-highfive
Jun 8, 2015
Collaborator
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.
If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.
Please see the contribution instructions for more information.
|
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. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
@bors r+ |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
|
added a commit
that referenced
this pull request
Jun 8, 2015
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bors
merged commit 93d01eb
into
rust-lang:master
Jun 8, 2015
brson
added
the
relnotes
label
Jun 16, 2015
neil-lobracco
reviewed
Jun 17, 2015
| // As a loose rule to avoid obviously incorrect suggestions, clamp the | ||
| // maximum edit distance we will accept for a suggestion to one third of | ||
| // the typo'd name's length. | ||
| let max_distance = std::cmp::max(name.len(), 3) / 3; |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
neil-lobracco
Jun 17, 2015
Because we're dividing the max(name.len(),3) by 3, isn't the max_distance always 1 now? It seems like we'd want this to be let max_distance = std::cmp::max(name.len() / 3, 3);, instead.
neil-lobracco
Jun 17, 2015
Because we're dividing the max(name.len(),3) by 3, isn't the max_distance always 1 now? It seems like we'd want this to be let max_distance = std::cmp::max(name.len() / 3, 3);, instead.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
niconii
Aug 6, 2015
Member
Nah, if it were min instead of max, it would be, but for example, if name.len() was 9, max(9,3) is 9, so max_distance would be 9 / 3 or 3.
niconii
Aug 6, 2015
Member
Nah, if it were min instead of max, it would be, but for example, if name.len() was 9, max(9,3) is 9, so max_distance would be 9 / 3 or 3.
neil-lobracco
reviewed
Jun 17, 2015
| @@ -3357,7 +3357,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { | ||
| NoSuggestion => { | ||
| // limit search to 5 to reduce the number |
fitzgen commentedJun 8, 2015
This makes the maximum edit distance of typo suggestions a function of the typo'd name's length. FWIW, clang uses this same hueristic, and I've found their suggestions to be better than rustc's. Without something like this, you end up with suggestions that aren't related at all when there are short variable names.
See also #20028 (comment)