feat(lsp): add related_information to vim.Diagnostic

[tom-anders]

Basically a simplified version of #19649 (comment)

Looking to get some initial feedback here. Instead of just appending to the message, we might instead want to store the related info in the vim.Diagnostic struct, and then instead extend the logic in vim.diagnostic.open_float()...?

This PR:

vscode:

EDIT: PR now only adds the related_information field to the toplevel of vim.Diagnostic (see discussion below). We could then decide in a follow-up PR if and how we should display this to the user.

[mfussenegger]

I think instead of extending the message this should go into the structure, otherwise users can't customise this. (And I'd wager there's a bunch who don't want this in the message).

There are also actions where we send the diagnostic back to the server and I think for the server to be able to re-associate the payloads with their internal state the diagnostic mustn't change.

[tom-anders]

I think instead of extending the message this should go into the structure, otherwise users can't customise this. (And I'd wager there's a bunch who don't want this in the message).

There are also actions where we send the diagnostic back to the server and I think for the server to be able to re-associate the payloads with their internal state the diagnostic mustn't change.

Sounds fair - I changed the PR so that now it only adds the relatedInformation field to the toplevel of vim.Diagnostic. We could then decide in a follow-up PR if and how we should display this to the user.

If this looks good, I'll add a test to diagnostic_spec.lua that covers this new field.

[gpanders]

vim.Diagnostic is not an "LSP diagnostic" though, and we try very hard to avoid imposing LSP specific concepts into the (generic) diagnostic structure. In this case it is probably fine since the "related information" seems pretty generic (could be used by non-LSP diagnostic producers).

If this does need to be in the top level structure it should follow the standard naming conventions (related_information, not relatedInformation). The latter style indicates that it is a field that maps directly to some LSP concept.

[tom-anders]

vim.Diagnostic is not an "LSP diagnostic" though, and we try very hard to avoid imposing LSP specific concepts into the (generic) diagnostic structure. In this case it is probably fine since the "related information" seems pretty generic (could be used by non-LSP diagnostic producers).

If this does need to be in the top level structure it should follow the standard naming conventions (related_information, not relatedInformation). The latter style indicates that it is a field that maps directly to some LSP concept.

Yeah to me this seems general enough (I could imagine something like overseer.nvim make use of this). Thanks for the hint related to naming conventions, fixed!

[tom-anders]

Thanks for the review! I extended the tests in lsp/diagnostic_spec.lua now.

[wookayin]

How about having a namescope, like vim.diagnostic.RelatedInformation?

[tom-anders]

Good point, done

[gpanders]

Suggested change

	• Add `vim.Diagnostic.related_information` field and fill it with
	`lsp.DiagnosticRelatedInformation[]`. Deprecate
	• Add `vim.Diagnostic.related_information` field and fill it with
	`vim.diagnostic.RelatedInformation[]`. Deprecate

[tom-anders]

I intentionally wrote lsp.DiagnosticRelatedInformation here though, to highlight the new logic I added in lsp/diagnostic.lua