Skip to content

Fix "did you mean" hinting #7895

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
Merged
merged 3 commits into from
Jan 7, 2020
Merged

Fix "did you mean" hinting #7895

merged 3 commits into from
Jan 7, 2020

Conversation

@odersky
Copy link
Contributor

@odersky odersky commented Jan 5, 2020 •
edited
Loading

The previous implementation was very obscure and wrong on multiple counts

  • It handled just definitions in the qualifier class itself instead of all of its members
  • It computed the Levenshtein distance wrongly
  • There might be other problems - the code was too convoluted to be able to tell what it did.

odersky added 2 commits January 5, 2020 18:33
@odersky
Refactor "did you mean" hinting
1752609 
 - Handle all members instead of just decls.
 - Simplify the logic and implementation.

To tell the truth, the previous implementation was too convoluted for me
to be able to say what it does exactly. But the new one seems to fit the
intuitive spec.
@odersky
Fix Levenshtein distance calculation
2c97437 
The previous algorithm was simply wrong, which went undiscovered because
there were no tests to its correctness.

Morale: Just because some code is purely functional does not mean it is
clear or correct.
@odersky odersky added the fasttrack Simple fix. Reviewer should merge or apply additional changes directly. label Jan 6, 2020
smarter
smarter previously requested changes Jan 6, 2020
View reviewed changes
@anatoliykmetyuk anatoliykmetyuk merged commit 5968d41 into scala:master Jan 7, 2020
@anatoliykmetyuk anatoliykmetyuk deleted the refactor-hints branch January 7, 2020 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@som-snytt som-snytt som-snytt left review comments

@smarter smarter smarter left review comments

+1 more reviewer

@anatoliykmetyuk anatoliykmetyuk anatoliykmetyuk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@anatoliykmetyuk anatoliykmetyuk

Labels

fasttrack Simple fix. Reviewer should merge or apply additional changes directly.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@odersky @smarter @som-snytt @anatoliykmetyuk