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

Add code actions for disabling a warning in the current file #1235

Merged
merged 7 commits into from
Jan 21, 2021

Conversation

georgefst
Copy link
Collaborator

@georgefst georgefst commented Jan 19, 2021

Previously known as haskell/ghcide#897.

Progress towards #705.

The properties tested were previously unnecessarily strong and would break witht the addition of irrelevant code actions. We now don't care about position and total quantity of code actions, only that the ones we care about exist.
@georgefst georgefst changed the title Disable warning action Add code actions for disabling a warning in the current file Jan 19, 2021
@jneira jneira mentioned this pull request Jan 20, 2021
7 tasks
@georgefst
Copy link
Collaborator Author

Fairly confident tests should pass now.

Are we squash-merging these days? Commit history here is a bit noisy due to some carelessness on my part...

@Ailrun
Copy link
Member

Ailrun commented Jan 20, 2021

@georgefst Mostly yes. Although some manual merges are done as normal merges, automated ones with "merge me" label are done as squash merges.

@georgefst
Copy link
Collaborator Author

... and we're done. Closes #571.

There are a lot of tests which rely on the exact position in which a particular action happens to appear in the list returned by getCodeActions. This is pretty brittle. I've rewritten the ones that were broken by this change, but we might want to take a look at improving the others.

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

Looks great, this is a nice feature and includes a solid test suite, thanks!

@jneira jneira added the merge me Label to trigger pull request merge label Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest adding -Wno-orphans when orphans are encountered
3 participants