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

Redundant imports/exports: use range only to determine which code actions are in scope #4063

Merged

Conversation

keithfancher
Copy link
Contributor

@keithfancher keithfancher commented Feb 9, 2024

...rather than doing a full compare with incoming Diagnostic objects from the client.

This brings the "remove redundant imports/exports" code actions more in line with behavior described in #4056, and has the pleasant side-effect of fixing broken code actions in neovim (#3857).

Manually tested removing redundant imports and exports in both neovim and VS Code -- all working as expected!

Rather than doing a full compare with incoming `Diagnostic` objects from
the client. This brings the "remove redundant imports/exports" code
actions more in line with behavior described in haskell#4056, and has the
pleasant side-effect of fixing broken code actions in neovim (haskell#3857).
@keithfancher
Copy link
Contributor Author

What do y'all think? Is this good to go, or any other open questions? Would probably be good for @michaelpj to take a look as well, to make sure I'm getting the intent of #4056 correct.

I'd love to get #3857 closed out, it's such an annoying bug for neovim users...

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@keithfancher
Copy link
Contributor Author

LGTM

Awesome, thank you!

As for getting it merged... do I just hang tight and wait for a maintainer to pull the trigger?

@fendor fendor added the status: needs review This PR is ready for review label Feb 21, 2024
@fendor fendor enabled auto-merge (squash) February 21, 2024 21:50
@michaelpj
Copy link
Collaborator

Sorry, I am going to take a look at this! I was on holiday last week and it hasn't quite got to the top of my pile yet :)

@keithfancher
Copy link
Contributor Author

Sorry, I am going to take a look at this! I was on holiday last week and it hasn't quite got to the top of my pile yet :)

Oh, no worries at all, take your time of course! I just wanted to be sure I wasn't missing some step on my end.

Thank you!

@fendor fendor merged commit af393d6 into haskell:master Feb 21, 2024
39 checks passed
soulomoon pushed a commit to soulomoon/haskell-language-server that referenced this pull request Feb 23, 2024
…ions are in scope (haskell#4063)

* Use *only* incoming range to determine which code actions are in scope

Rather than doing a full compare with incoming `Diagnostic` objects from
the client. This brings the "remove redundant imports/exports" code
actions more in line with behavior described in haskell#4056, and has the
pleasant side-effect of fixing broken code actions in neovim (haskell#3857).

* Remove redundant imports ;)

* Rename param for clarity

---------

Co-authored-by: fendor <fendor@users.noreply.github.com>
Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me btw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs review This PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants