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.

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

Hlint: CodeAction with isPreferred #3018

Merged

Conversation

andys8
Copy link
Collaborator

@andys8 andys8 commented Jul 5, 2022

A quick fix should be marked preferred if it properly addresses the underlying error. A refactoring should be marked preferred if it is the most reasonable choice of actions to take.

For hlint it is the likeliest choice to apply the fix. Ignore the rule
for the complete module is preferred less.

Related to #2057

Demo

The client - in my case Vim/Coc - can respect isPreferred. In case of CoC it will be reflected in the sort order of code actions.

Before

screenshot-1657034414

After

screenshot-1657034612

@michaelpj
Copy link
Collaborator

Please include the rationale for why we're making this decision as a comment in the code.

I think your argument is good, and probably extends to other code actions that are produced directly from diagnostics. The "add pragma" code action seems like another good candidate.

I'm not totally sure what our rubric should be, though. Arguably ignoring the hint also clearly deals with the diagnostic, maybe both should be preferred? I don't know. I think it's reasonable to assume that if people have the hint enabled globally they probably want to fix it rather than ignore it.

@andys8
Copy link
Collaborator Author

andys8 commented Jul 5, 2022

Please include the rationale for why we're making this decision as a comment in the code.

Sure thing 2a9a687

I think it's reasonable to assume that if people have the hint enabled globally they probably want to fix it rather than ignore it.

That would also be my main argument. I know the project needs to work for different projects and developers, but the main influence of isPreferred seems to express the "default" quick fix which in case of an installed and configured hlint would be to apply the quick fixes, rather than ignoring them all the time. My assumption is since where talking mostly about sort order this can save a bunch (millions 😄) of keystokes.

andys8 added 2 commits July 5, 2022 19:26
> A refactoring should be marked preferred if it is the most reasonable choice of actions to take.

For hlint it is the likeliest choice to apply the fix. Ignore the rule
for the complete module is preferred less.
Including the rationale for why we're making this decision as a comment in the code.
@andys8 andys8 force-pushed the improvement/hlint-codeaction-is-preferred branch from 2a9a687 to ab7f57b Compare July 5, 2022 17:26
@michaelpj michaelpj merged commit 1bc1def into haskell:master Jul 6, 2022
hololeap pushed a commit to hololeap/haskell-language-server that referenced this pull request Aug 26, 2022
* Hlint: CodeAction with isPreferred

> A refactoring should be marked preferred if it is the most reasonable choice of actions to take.

For hlint it is the likeliest choice to apply the fix. Ignore the rule
for the complete module is preferred less.

* Hlint: Comment for isPreferred

Including the rationale for why we're making this decision as a comment in the code.
andys8 added a commit to andys8/haskell-language-server that referenced this pull request Sep 15, 2022
`isPreferred` can influence the client side order of code actions. The
idea is that an unused import is likely to be removed and less likely
the warning will be disabled. Therefore actions to remove a single or
all redundant imports should be prefered, so that the client can
prioritize them higher.

Followup of
<haskell#3018>
andys8 added a commit to andys8/haskell-language-server that referenced this pull request Sep 15, 2022
`isPreferred` can influence the client side order of code actions. The
idea is that an unused import is likely to be removed and less likely
the warning will be disabled. Therefore actions to remove a single or
all redundant imports should be preferred, so that the client can
prioritize them higher.

Followup of
<haskell#3018>
andys8 added a commit to andys8/haskell-language-server that referenced this pull request Sep 15, 2022
`isPreferred` can influence the client side order of code actions. The
idea is that an unused import is likely to be removed and less likely
the warning will be disabled. Therefore actions to remove a single or
all redundant imports should be preferred, so that the client can
prioritize them higher.

Followup of
<haskell#3018>
andys8 added a commit to andys8/haskell-language-server that referenced this pull request Sep 15, 2022
`isPreferred` can influence the client side order of code actions. The
idea is that an unused import is likely to be removed and less likely
the warning will be disabled. Therefore actions to remove a single or
all redundant imports should be preferred, so that the client can
prioritize them higher.

Followup of
<haskell#3018>
andys8 added a commit to andys8/haskell-language-server that referenced this pull request Sep 16, 2022
`isPreferred` can influence the client side order of code actions. The
idea is that an unused import is likely to be removed and less likely
the warning will be disabled. Therefore actions to remove a single or
all redundant imports should be preferred, so that the client can
prioritize them higher.

Followup of
<haskell#3018>
pepeiborra pushed a commit that referenced this pull request Sep 18, 2022
`isPreferred` can influence the client side order of code actions. The
idea is that an unused import is likely to be removed and less likely
the warning will be disabled. Therefore actions to remove a single or
all redundant imports should be preferred, so that the client can
prioritize them higher.

Followup of
<#3018>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants