add a client option for hiding non-project diagnostics #2218
Conversation
rchl
changed the title
docs/src/client_configuration.md
Outdated
| @@ -42,6 +42,7 @@ Below is an example of the `LSP.sublime-settings` file with configurations for t | |||
| | initializationOptions | options to send to the server at startup (rarely used) | | |||
| | selector | This is _the_ connection between your files and language servers. It's a selector that is matched against the current view's base scope. If the selector matches with the base scope of the the file, the associated language server is started. For more information, see https://www.sublimetext.com/docs/3/selectors.html | | |||
| | priority_selector | Used to prioritize a certain language server when choosing which one to query on views with multiple servers active. Certain LSP actions have to pick which server to query and this setting can be used to decide which one to pick based on the current scopes at the cursor location. For example when having both HTML and PHP servers running on a PHP file, this can be used to give priority to the HTML one in HTML blocks and to PHP one otherwise. That would be done by setting "feature_selector" to `text.html` for HTML server and `source.php` to PHP server. Note: when the "feature_selector" is missing, it will be the same as the "document_selector". | |||
| | show_non_project_diagnostics | By default all diagnostics are ignored for files that are not within the current project (window) folders. Set this to `true` to see diagnostics even if the file is not within the project folders. If project (window) has no folders then diagnostics are never ignored, regardless what this option is set to. | | |||
There was a problem hiding this comment.
How about we just remove this feature altogether?
There was a problem hiding this comment.
In the sense of always show diagnostics for files outside the project folders? Why would you want to see diagnostics for 3rd-party libraries or standard libraries?
I would even suggest to set files outside the project folders to read-only, if they were opened via a LSP feature (Goto Definition, ...), to prevent accidentally editing them.
There was a problem hiding this comment.
I think I'm fine with that as long as we still support ignoring based on file ignore patterns
There was a problem hiding this comment.
You’re right @jwortmann, this is indeed the original goal. But this setting does complicate things. Shouldn’t it be up to the server to decide to show diagnostics for 3rd party libs?
There was a problem hiding this comment.
How do other LSP clients handle this?
There was a problem hiding this comment.
Shouldn’t it be up to the server to decide to show diagnostics for 3rd party libs?
Hm... maybe, but I'm unsure.
Julia server does indeed filter out files that are not contained within the workspace:
https://github.com/julia-vscode/LanguageServer.jl/blob/4d40ca67d2a50485a04f42823b76302407249e42/src/requests/textdocument.jl#L209
However, Pyright by default does report diagnostics for such files. According to microsoft/pyright#603 (comment) it is possible to avoid this by setting "python.analysis.typeCheckingMode": "off" and then override this setting via a pyrightconfig.json file inside the project folder. That seems quite annoying to me, if I have to do it separately for each project, and also if the pyrightconfig.json files are included in git (e.g. this repo).
I don't know about other servers, but I doubt this is handled well or configurable by all servers. I don't have objections against the setting from this PR, but I think to remove the option to hide diagnostics outside the workspace folders alltogether would be a usability regression.
There was a problem hiding this comment.
What about adding this but showing non-project diagnostics by default
There was a problem hiding this comment.
I'm fine with that too (would rename the option to hide-non-project-diagnostics then so that the default is false).
What do others think?
There was a problem hiding this comment.
a) show_non_project_diagnostics: default False - good. this is backwards compatible
b) show_non_project_diagnostics: default True - good. it introduces a change in behavior
c) hide_non_project_diagnostics, default False - good. it introduces a change in behavior
d) hide_non_project_diagnostics, default True - good, this is backwards compatible
e) removing the setting and always showing the diagnostics reported - I could live with it, it introduces a change in behavior some people might not like it.
If I had to choose, I would choose c).
I would suggest always displaying the diagnostic reported by the server,
but allow the user to hide diagnostics if they do not find them useful.
There was a problem hiding this comment.
Yes after some thought it’s probably best to go with a Boolean option. Whether it should disable/enable non-project diagnostics by default is up to you guys.
There was a problem hiding this comment.
I wanted to ask if it made more sense just to have a setting hide_non_project_diagnostics in LSP.sublime-settings file... because maybe people would expect to find that setting there.
But I really like the granularity that it can be configured per server configuration more.
Maybe add an example in the breaking changes to better illustrate what "server-specific configuration" means?
Diagnostics for files that are not withing the project folders are no longer ignored.
You can sethide_non_project_diagnosticsin server-specific configuration to enable old behavior.
// LSP-pyright.sublime-settings
{
"hide_non_project_diagnostics": false,
}
|
I think it needs to be per-server since some servers like LSP-json are not really project aware and it makes sense to never ignore diagnostics for those (IMO) while for other it might just add noise when used outside of project boundaries. |
rchl
changed the title
rchl
changed the title
|
Still needs update of parameters in Line 34 in ca101eb |
|
Thanks. Pushed to master. |
* main: Clear pull diagnostics on file closed (#2224) html-escape diagnostic-related strings (#2228) Allow style overrides for inlay_hints.css (#2232) Fix exception for null response id (#2233) Add "outline" as an option for "document_highlight_style" (#2234) remove accidentally committed files Improve label detail support in completions (#2212) Update clojure-lsp docs (#2226) Add support for pull diagnostics (#2221) update test MockManager after API change add a client option for hiding non-project diagnostics (#2218) Fix some features might not work with dynamical registration (#2222)
Resolves #1835