add diagnostic dynamic long description

microsoft / language-server-protocol

Defines a common protocol for language servers.

https://microsoft.github.io/language-server-protocol/

Creative Commons Attribution 4.0 International

10.95k stars 766 forks source link

add diagnostic dynamic long description #1825

Open RobbyCBennett opened 9 months ago

RobbyCBennett commented 9 months ago

https://github.com/microsoft/language-server-protocol/issues/1461

RobbyCBennett commented 9 months ago

@microsoft-github-policy-service agree company="Scientific Toolworks, Inc."

dbaeumer commented 9 months ago

@RobbyCBennett tahnks for the PR. Changing the spec is the easy part :-). To contribute additional functionality to the spec a reference implementation is needed as well. See https://github.com/microsoft/language-server-protocol/blob/main/contributing.md

I will keep the PR open for a while but without a reference implementation I will not be able to merge the PR.

jwortmann commented 9 months ago

Isn't this a breaking change?

Currently, the href field is mandatory. With this PR, it would become optional. So any client which unconditionally tries to read href might crash if the field doesn't exist.

RobbyCBennett commented 9 months ago

To contribute additional functionality to the spec a reference implementation is needed as well.

Thank you. I was just looking to get some feedback before the reference implementation is made.

Isn't this a breaking change?

Yes, I will change it to accommodate for language clients that will crash after assuming the href field is defined.

Perhaps instead, it could be simplified to be part of Diagnostic, as a new field longMessage that could be just below message. I think that this would be more clear and it also wouldn't break language clients.

https://github.com/microsoft/language-server-protocol/pull/1825/commits/1348fc7c74f7427eb9376526725da673bd1e45ce

RobbyCBennett commented 9 months ago

I didn't follow https://github.com/microsoft/language-server-protocol/blob/main/contributing.md because it seems to be intended for adding new types and methods.

Reference implementation:

Step 1: (this PR)

Add messageDetails field to Diagnostic

Step 2: PR in VSCode repo:

Add messageDetails field to Diagnostic (this is a different Diagnostic interface, defined in src/vscode-dts/index.d.ts)

Step 3

Update the projects that get their 2 Diagnostic types from these 2 repos (like the npm module @types/vscode), which should prepare for step 4.

Step 4 (PR in VSCode Language Server Node repo):

Change asDiagnostic function in createConverter and add asMessageDetails function (in client/src/common/codeConverter.ts)
Change asDiagnostic function in createConverter and add asMessageDetails function (in client/src/common/protocolConverter.ts)