Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DiagnosticTag Type #500

Closed
mjbvz opened this issue Jun 19, 2018 · 8 comments
Closed

Add DiagnosticTag Type #500

mjbvz opened this issue Jun 19, 2018 · 8 comments
Labels
feature-request Request for new features or functionality
Milestone

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Jun 19, 2018

VS Code recently added a DiagnosticTag type for attaching additional metadata to a diagnostic. These tags can be used by editors to control the presentation of diagnostics. See microsoft/vscode#51104 for a discussion of this

Currently only one tag type has been added on the vs code side: DiagnosticTag.Unnecessary. This tag makes diagnostics for unused variables or unreachable bits of code. VS Code renders code marked with DiagnosticTag.Unnecessary as faded out.

DiagnosticTag only adds metadata to a diagnostic and should not break clients that do not support it. With DiagnosticTag.Unnecessary for example, clients that do not support tags will still receive standard diagnostics for unused variables and can render them as regular errors / warnings / suggestions

@mjbvz mjbvz changed the title Add DiagnosticTag Add DiagnosticTag Type Jun 19, 2018
@dbaeumer
Copy link
Member

This is more complicated for the LSP. Since the LSP is implemented for different clients simply adding this without thinking about how it will be implemented on other clients is not what we should do. We at least do need flag this as a client capability. BTW: this is why I was not a big fan of adding this that way.

@dbaeumer dbaeumer added this to the Backlog milestone Jun 20, 2018
@dbaeumer dbaeumer added the feature-request Request for new features or functionality label Jun 20, 2018
@dbaeumer
Copy link
Member

Another thing we need to consider is that Diagnostics flow in both ways. So even if a client doesn't handle a DiagnosticTag we need to spec that the client tries to preserve the information when a diagnostic object is send from the client to the server in a code action request.

@mjbvz
Copy link
Contributor Author

mjbvz commented Jun 20, 2018

Clients that do not support DiagnosticTag should still be able to consume the diagnostics, they just will miss out on the special rendering of those tags. For example, TS sends all unused variable as suggestion level diagnostics. A client that does not know about DiagnosticTag will render them as normal inline suggestions

@dbaeumer
Copy link
Member

dbaeumer commented Jun 21, 2018

I agree, but the problem I have in the LSP are objects that flow in both ways and the fact that the LSP tries to work in a backwards compatible way and for different client implementation. So in general that means:

  • if we had an enum, tag value client announce their supported value sets to avoid that servers do compute expensive values which the client drops anyways.
  • if an object flows in both ways then we need to let the server know if the client preserves the values even if he doesn't honor them. Assume that a server generates a diagnostic with a tag and the diagnostic comes back to the server without that tag in a code action request the server might be surprised.

This is all not an issue with the VS Code API since we have strict versioning which due to the loose Eco system of LSP we tried to avoid (btw debug does the same there with its protocol). So whenever we add language API to VS Code I try to do that in a way that makes it not too hard to support in LSP.

@rcjsuen
Copy link
Contributor

rcjsuen commented Apr 9, 2019

Looks like this got added into vscode-languageserver-node.

See microsoft/vscode-languageserver-node#477.

@KamasamaK
Copy link
Contributor

Pending PR is #645

@dbaeumer
Copy link
Member

Yes, but only in a next version so far.

@dbaeumer
Copy link
Member

Updated spec on dbaeumer/3.15 branch.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

4 participants