microsoft / language-server-protocol

Defines a common protocol for language servers.
https://microsoft.github.io/language-server-protocol/
Creative Commons Attribution 4.0 International
11.25k stars 799 forks source link

Clarify how to dynamically register for workspace/diagnostic requests #1723

Open 0xJonas opened 1 year ago

0xJonas commented 1 year ago

As far as I can tell, when a server wants to dynamically register for workspace/diagnostic requests, it needs to do so by registering for textDocument/diagnostic and setting DiagnosticOptions.workspaceDiagnostics to true. At least that is how the implementation in vscode-languageserver-node seems to handle it. If this is correct and intentional, it would be helpful to have a note in the spec for workspace/diagnostic for this counter-intuitive behavior for dynamic registration, i.e. that the Registration.method actually has to be textDocument/diagnostic instead of workspace/diagnostic.

Also, a registrationMethod field in definition for workspace/diagnostics in the meta-model may need to be added for this.

dibarbet commented 6 months ago

I hit an issue related to this + the identifier in DiagnosticOptions - https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticOptions in the C# extension.

It currently doesn't seem possible to register separate identifiers for workspace diagnostics. Since I have to register the workspace capability via textDocument/diagnostic, it ends up registering that identifier for document pull as well as workspace pull.

This is an issue for us - generally we have multiple different identifiers for document pull for different buckets of analysis (compiler syntax / compiler semantic / analyzer syntax / analyzer semantic / hot reload). This prevents for example a slow semantic analyzer from blocking document compiler syntax diagnostics from showing up, and used to bucket diagnostics with different lifetimes (hot reload).

However, for workspace diagnostics we don't need to split the diagnostics into as many different categories. I'd rather just have one or two workspace pull buckets. But currently registering identifiers for workspace pull diagnostics also registers them for document pull diagnostics.

Right now my workaround is to ignore workspace 'identifier's in doc pull on the server, but we still get the unnecessary requests.

dbaeumer commented 6 months ago

@dibarbet just to ensure I understand you correctly. You want to register for workspace pull without document pull. Right?

If this is the case and for symmetrical issue we could have a registration path workspace/diagnostics with the following options

{
    identifier?: string;
    interFileDependencies: boolean;
    documentDiagnostics: boolean;
}

This would also help with @0xJonas issue.

dibarbet commented 6 months ago

@dibarbet just to ensure I understand you correctly. You want to register for workspace pull without document pull. Right?

Yup, we want to register specific indentifiers only for workspace pull.

That proposal would work for us I think - though it does feel a bit weird to have the documentDiagnostics boolean there. If an identifier needs both doc and workspace it can already register via the text document registration version. But maybe it makes sense for symmetry.

dbaeumer commented 6 months ago

though it does feel a bit weird to have the documentDiagnostics

I will think about leaving it out.