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

Document diagnostics report not used when text document closed #1013

Closed
JohnnyMorganz opened this issue Jun 24, 2022 · 6 comments
Closed

Document diagnostics report not used when text document closed #1013

JohnnyMorganz opened this issue Jun 24, 2022 · 6 comments
Labels
info-needed Issue requires more information from poster
Milestone

Comments

@JohnnyMorganz
Copy link

Using language client 8.0.2-next.5. The server supports pull document diagnostics, but not workspace diagnostics.

The client (correctly) sends textDocument/diagnostic to pull a new report when a file is closed (i.e., is no longer managed).
However, the returned report by the server is not then used by the client - the old diagnostics still remain.

In my case, I want to clear the diagnostics of some files when they are closed / no longer managed, but this does not happen.

Minimal example: https://github.com/JohnnyMorganz/pull-diagnostics-bug

@dbaeumer
Copy link
Member

@JohnnyMorganz thanks for the GitHub repository. I was able to reproduce this, however we need to think a little what the expected behavior would be in such a case. I opt to do the following:

  • if the server supports workspace diagnostics then leave the diagnostics as is. The one from the workspace diagnostics will then catchup with any changes.
  • if the server doesn't support workspace diagnostics we should not pull again for the document. Major reason is that pull doesn't pull for other closed documents. The client should instead simply clear the diagnostics for the document to bring it into the same state as if the document never got opened.

@aeschli since you are using pull now as well any ideas / advice here?

@dbaeumer dbaeumer added the info-needed Issue requires more information from poster label Jun 27, 2022
@JohnnyMorganz
Copy link
Author

JohnnyMorganz commented Jun 27, 2022

Thanks for checking it out!

if the server doesn't support workspace diagnostics we should not pull again for the document. Major reason is that pull doesn't pull for other closed documents. The client should instead simply clear the diagnostics for the document to bring it into the same state as if the document never got opened.

For my use case, I only want to clear diagnostics for files which have explicitly been marked as "ignored". i.e., if a user opens an ignored file, we show diagnostics whilst its open (to highlight any problems), but then once they close it we remove the diagnostics so it no longer clutters their UI. For non-ignored files, I want to keep the diagnostics there as it may be useful to the user (e.g., type errors). For us, ignored-files are typically third party files which is why a user doesn't really need to see the diagnostics generally, but it may be helpful if they are trying to debug something.

I do intend to personally switch to workspace diagnostics some time, but currently we don't have the infrastructure set up yet in our server to support it. I just wanted to point out my use case as it may be more worthwhile allowing the language server to control what should happen to the diagnostics, rather than blanketly clearing them.

EDIT: another use case in mind is instead of clearing all diagnostics, a server may only want to show a subset that are of high importance once the file is closed. The client doesn't apply the diagnostics returned in the report in this case as well

@aeschli
Copy link
Contributor

aeschli commented Jun 30, 2022

It is the client that decides when to show (or clear) diagnostics. VS Code only shows document diagnostics for documents that are open in an editor, but other editors might do this differently. The language server basically just replies to requests: Compute all diagnostics for a document content. That document might be open in an editor, but could also be a virtual document, used programmatically.

@dbaeumer
Copy link
Member

dbaeumer commented Jul 1, 2022

I would basically implement the clear on the client side. Does that make sense to you @aeschli

@JohnnyMorganz
Copy link
Author

JohnnyMorganz commented Jul 1, 2022

Understandable that the client should be controlling when it shows diagnostics.
I guess my main ask would be is there any way of controlling this as a user, not necessarily as a server?

I feel like its quite important, as a user, that some diagnostics should be made available even when the file is closed (microsoft/vscode#13953 shows the importance of this for js/ts), whilst other files can be ignored.
It seems that this is something wanted by other servers too (#848 (comment)), where there are cases for errors to only be shown whilst a file is open for some (not necessarily all) files

Maybe this is out of scope for this repository/issue though. Its probably also possible to control this once we implement workspace diagnostics, unless that is also not really recommended there.

EDIT: maybe this could be something configurable by a predicate in DiagnosticPullOptions on the client? e.g. as an extension to filter(document, mode), further including some sort of info about the state (open/closed)

@aeschli
Copy link
Contributor

aeschli commented Jul 4, 2022

@JohnnyMorganz You should looking into using workspace diagnostics

  • Workspace diagnostics are shown for all resources, unless the ones open in an editor
  • Document diagnostics are shown for the resources in open editors.
    Your workspace diagnostics would not contain diagnostics for ignored resources, but the your document diagnostics would.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

3 participants