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

fix: clear diagnostics on cancel unconditionally #18858

Merged

Conversation

davidbarsky
Copy link
Contributor

This should fix #18854.

This is just an impl of Lukas's comment; I tested this on rust-analyzer, canceled flycheck while it was running, and saw diagnostics get cleared when they previously wouldn't.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 6, 2025
@ChayimFriedman2
Copy link
Contributor

The bug will only get triggered when diagnostics are not cancelled AFAIK.

Anyway I tested this and this works.

@ChayimFriedman2 ChayimFriedman2 added this pull request to the merge queue Jan 6, 2025
Merged via the queue into rust-lang:master with commit ccc7468 Jan 7, 2025
9 checks passed
@davidbarsky davidbarsky deleted the davidbarsky/fix-18854 branch January 7, 2025 00:50
@contextref
Copy link

contextref commented Jan 7, 2025

As a general policy, is there a need for a test so that future regressions of this nature are prevented?

(And thank you for the prompt fix!)

@ChayimFriedman2
Copy link
Contributor

@contextref In general we do require tests, but flycheck specifically is a part that is hard to test.

@davidbarsky
Copy link
Contributor Author

To expand on what Chayim said, we'd need to rewrite flycheck to make it more testable. We probably should, but given the widespread nature of the breakage, that'll need to be a "later" thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange errors as if the extension forget to execute the linter
4 participants