-
Notifications
You must be signed in to change notification settings - Fork 196
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
published diagnostics are discarded due to file version #1024
Comments
Version documentation is rather scarce https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#publishDiagnosticsParams
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_publishDiagnostics the spec does not mention anything about client rejecting diagnostics. I wonder if helix validation isn't too strict. Otherwise race conditions are inherent to diagnostic publish. Code parsing and compilation happens asynchronously and document updates can happen in between start of the parsing and document publish. Can you post log including textDocument/didChange notifications where update for version x + 1 happens before diagnostic for version x is published? I'd like to be sure there are no obvious bugs on ElixirLS side. |
Notice that in
|
Yeah I think that's what's going wrong, because this PR fixes the issue: https://github.com/helix-editor/helix/pull/8760/files Let's see what the feedback on that PR will be |
Wow that was fast: helix-editor/helix#8760 (comment) |
Here is an example session
I tagged you along in that helix issue by the way, hope you don't mind, I really want to get to the bottom of this 😄 |
This does look like a bug on elixirls side, doesn't it? if you look at these lines:
Here the version sent with didChange is 1 but we respond with 2? Shouldn't there always be a corresponding publishDiagnostics? |
Thanks for detailed report @dvic. This is indeed an ElixirLS issue. The versioning was broken since the original code dump (or it may got broken when support for LSP 3.0 was added ca 2017). We first store the version in
This bug started manifesting in v0.17 when we started sending diagnostics with version. I have a fix ready on mu dev branch and plan to release it in v0.18 |
…after all changes applied This was broken since the original code drop (or may be an artefact of LSP 2.0) Fixes #1024
…after all changes applied This was broken since the original code drop (or may be an artefact of LSP 2.0) Fixes #1024
In the Helix editor I'm seeing published diagnostics being discarded because of a version mismatch. I see that there is a todo in the computation of the version:
elixir-ls/apps/language_server/lib/language_server/server.ex
Lines 1475 to 1487 in 987b4de
Could this be the reason of the version mismatch in the Helix editor? If so, should we have the option to disable sending the version? (it's optional, right?)
context: helix-editor/helix#8754
The text was updated successfully, but these errors were encountered: