-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add an option to disable diagnostics #5682
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
Conversation
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.
Is there a way to configure this in VS Code with this PR?
I don't think so. I initially thought that it will be two different features, and thus have to be implemented separately. If you'll give me a clue where the extension provides a configuration for a server, I can implement it (either as a part of this PR, or as a different PR). |
Co-authored-by: Jonas Schievink <[email protected]>
You need to add the new setting to editors/code/package.json, and that should be it |
@jonas-schievink Is there anything else that has to be done within this PR? |
crates/ide/src/lib.rs
Outdated
#[derive(Debug, Default, Clone)] | ||
pub struct AnalysisConfig { | ||
pub disabled_diagnostics: HashSet<String>, | ||
} |
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.
@matklad is it okay to make this config structure part of AnalysisHost
or should it be handled differently?
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.
Good question! Given that we only do surface filtering at the moment, I think it's better to just pass it in as an argument to Analysis::diagnostics
.
If, in the future, we decide to disable diagnostics on a more fundamental level (ie, avoid computing them at all), I think we'll have to make this an input of the database.
But for now, adding an argument seems like a better approach.
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.
Sure, will do that.
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.
Done.
bors r+ |
As far as I know, currently it's not possible to disable a selected type of diagnostics provided by
rust-analyzer
.This causes an inconvenient situation with a false-positive warnings: you either have to disable all the diagnostics, or you have to ignore these warnings.
There are some open issues related to this problem, e.g.: #5412, #5502
This PR attempts to make it possible to selectively disable some diagnostics on per-project basis.