-
-
Notifications
You must be signed in to change notification settings - Fork 414
Use an importance score to order the suggested import code action #3271
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
Use an importance score to order the suggested import code action #3271
Conversation
7727c92 to
fc7d687
Compare
|
Hi! Please include a test, or an example how the behaviour is now improved! (I've seen how much it is improved, but we want to document it for everyone else as well) |
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
Outdated
Show resolved
Hide resolved
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
Outdated
Show resolved
Hide resolved
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
Outdated
Show resolved
Hide resolved
e30fdb8 to
8aece66
Compare
8aece66 to
f6f1c83
Compare
fendor
left a comment
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.
LGTM, awesome!
michaelpj
left a comment
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.
Were we going to get some tests?
| data ImportSuggestion = ImportSuggestion !Int !CodeActionKind !NewImport | ||
| deriving ( Eq ) | ||
|
|
||
| instance Ord ImportSuggestion where |
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 think this Ord instance is not lawful because it doesn't compare the CodeActionKind, which is taken into account by the Eq instance.
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.
Thanks for catching this! I will fix this ASAP
CodeAction,NewImport) into anImportSuggestionImportSuggestionfirst based on the score in descending order and second based on the import module names in alphabetic orderBefore the PR changes:
After the PR changes: