Diagnostic pulling could support multiple identifiers to allow for two stage diagnostic retrieval

[rchiodo]

Right now pull diagnostics are registered with a single identifier per server:

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_pullDiagnostics

export interface [DiagnosticOptions](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticOptions) extends [WorkDoneProgressOptions](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workDoneProgressOptions) {
	/**
	 * An optional identifier under which the diagnostics are
	 * managed by the client.
	 */
	identifier?: string;

This limits a server to having to handle all diagnostics through a single pull request.

If multiple identifiers were supported, the server could split its diagnostic computations into different chunks, for example, syntax errors and semantic errors.

I'm proposing a new optional field for DiagnosticOptions:

/**
 * Diagnostic options.
 *
 * @since 3.17.0
 */
export interface [DiagnosticOptions](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticOptions) extends [WorkDoneProgressOptions](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workDoneProgressOptions) {
	/**
	 * An optional identifier under which the diagnostics are
	 * managed by the client.
	 */
	identifier?: string;

	/**
	 * Whether the language has inter file dependencies meaning that
	 * editing code in one file can result in a different diagnostic
	 * set in another file. Inter file dependencies are common for
	 * most programming languages and typically uncommon for linters.
	 */
	interFileDependencies: boolean;

	/**
	 * The server provides support for workspace diagnostics as well.
	 */
	workspaceDiagnostics: boolean;


        /*
         * Additional identifiers that might be returned by the server
         */
       extraIdentifiers?: string[]
}

This could allow multiple diagnostic collections per server, which I think would be implemented here:

vscode-languageserver-node/client/src/common/diagnostic.ts

Lines 420 to 421 in 8e62556

	this.diagnostics = Languages.createDiagnosticCollection(options.identifier);
	this.openRequests = new Map();

[dbaeumer]

Actually the idea is that additional pull providers are registered dynamically. So each registration can have its own identifier and set of attributes. Would that work for you?

[rchiodo]

I must be doing the registration wrong then. I'm returning it in my server capabilities:

        if (this.client.hasPullDiagnosticsCapability) {
            result.capabilities.diagnosticProvider = {
                identifier: 'pylance',
                documentSelector: null,
                interFileDependencies: true,
                workspaceDiagnostics: initializationOptions.diagnosticMode === 'workspace',
            };
        }

Or are you saying there's a secondary step I can do after that to add more?

[rchiodo]

I think this issue is a dupe of microsoft/language-server-protocol#1088, which I believe is asking for the same thing?

[rchiodo]

Ah @heejaechang pointed out we could do it this way:
https://github.com/microsoft/pyrx/blob/main/packages/pyright/packages/pyright-internal/src/languageServerBase.ts#L1415

The spec says that each dynamic registration has to be a different document selector though.

Is that true?

[dbaeumer]

I think this issue is a dupe of microsoft/language-server-protocol#1088, which I believe is asking for the same thing?

That request was for the push diagnostics. It basically got added for pull with the identifier.

[dbaeumer]

Ah @heejaechang pointed out we could do it this way:
https://github.com/microsoft/pyrx/blob/main/packages/pyright/packages/pyright-internal/src/languageServerBase.ts#L1415

This is what I meant with dynamic registration.

Having two for the same selector for pull is fine since the identifier is part of the params. Otherwise you couldn't distinguish them. I will add something to the spec.

[rchiodo]

Thanks, sounds like it will work then.