Support textDocument/diagnostic specification (Pull diagnostics)

[SofusA]

Closes #7757, if workspace diagnostics is not a requirement. I have not yet found a language server which supports this capability.

Handling of response was originally done by @woojiq in #7900. I was not able to include their git history since their branch has been deleted.

Tested with language servers:

• eslint version >4.10.0
• roslyn language sever for C# version >4.12.0
• ruby-lsp
• rust-analyzer version >2025-03-03

Diagnostics are pulled when a document is changed for each language server with pull diagnostics feature with an async hook after a debounce period.

Diagnostics are also pulled when a document is opened, when changed to buffer and when a language server is successfully initiated. This is done without debounce.

[SofusA]

Thanks again for the reviews!
I added your comments and did some refactoring. I closed the conversations, i am sorry if it is usually the reviewer who does this.

The request is currently only being send on document changes, which makes a terrible user experience 😄

Is there a way to send this request when a document is opened, or should a workspace diagnostic event be send when the language server (which supports this feature) is started?

Edit: By the way, i am using this branch daily for C# development, since it allows me to use the same language server as the C# vscode extension. So i am keeping the branch up to date with master branch. Let me know if the merge commits is making too much noise.

[SofusA]

I did some experiments with this on roslyn language server. It might be that this language server just behaves weird (which it does on other stuff), but sending workspace/diagnostic after the server is initiated always resulted in and empty diagnostic list. Also with some seconds delay.

I also noticed that responses can either be a full or an related unchanged document report. At the moment, since we only listening on changes, we only get the full reports. This means that when i eg. rename something, i will not see diagnostics on related documents until i make a change.

I am not sure if the specification suggest to pull diagnostics every time a document is viewed, or if we should pull diagnostics for all open documents on changes to a document.

Edit: Pulling diagnostics for all open documents seems to work fine actually.

[SofusA]

My plan is to send pull diagnostics request:

1. When current document is edited for all open documents, if possible where other documents has same active language server by id as edited document
2. When a new document is opened

[SofusA]

Edit: This is no outdated.
I now pull diagnostics when a document has changed for that document for each language server with pull diagnostics feature. This is done with a async hook with a debounce.

I also pull blocking when a document is opened or changed to buffer.

I chose this model because i thought pulling for all open documents on edit was too aggressive.

I have now added two events: *Pull diagnostics for documents* Trigger: When documents are opened Document ids are collected in a hashset After a short debounce all documents pull from all its language servers with pull diagnostics support.

Pull diagnostics for language servers
Trigger: When document are saved
Language server ids with pull diagnostics support from saved document are collected in a hashset.
After a longer debounce all language servers for all open document are pulled

Document ids are being passed around, and i do some cloning of ids.

Overall it appears to work well. At least better than the first implementation with idle timeout.

[lizclipse]

I've been trying this PR to see if it'll solve some issues using the ESLint LS from vscode-langservers-extracted, and I've run into a couple of bugs. Firstly, if you open a file directly using the CLI args, then the diagnostics won't show, and instead you have to close the buffer and re-open via the file picker. Secondly, and most noticable I think, if I then close this buffer and try to re-open it again then I get a crash:

thread 'main' panicked at helix-term/src/handlers/diagnostics.rs:144:19:
no entry found for key
stack backtrace:
   0: rust_begin_unwind
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_fmt
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:72:14
   2: core::panicking::panic_display
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:168:5
   3: core::panicking::panic_str
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:152:5
   4: core::option::expect_failed
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/option.rs:1988:5
   5: helix_term::handlers::diagnostics::pull_diagnostics_for_documents
   6: helix_term::job::Jobs::handle_callback
   7: helix_term::application::Application::run::{{closure}}
   8: tokio::runtime::park::CachedParkThread::block_on
   9: tokio::runtime::runtime::Runtime::block_on
  10: hx::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Of note, you do have to be using vscode-langservers-extracted@>=4.9.0 (I'm testing on 4.10.0), as that's when they changed something to stop the LS working originally, and is why I'm trying this.

[SofusA]

Thanks for testing!

Firstly, if you open a file directly using the CLI args, then the diagnostics won't show, and instead you have to close the buffer and re-open via the file picker.

I see the same behaviour with roslyn language server. I think this is because we are requesting diagnostics before the language server is ready. I was hoping that this would be the language servers responsibility. I am not sure what to do about this.
Diagnostics will be requested if you make a change to the document

Secondly, and most noticable I think, if I then close this buffer and try to re-open it again then I get a crash

I fixed this by skipping the diagnostics request if the document id is not in the editor.
I am not sure if this is the best solution.

Edit:

Of note, you do have to be using vscode-langservers-extracted@>=4.9.0 (I'm testing on 4.10.0), as that's when they changed something to stop the LS working originally, and is why I'm trying this.

I am actually (without knowing) been using vscode-eslint-language-server version 4.10.0 on this branch. It is working for me with pull diagnostics. @lizclipse Can you check if it work with my latest changes?

[lizclipse]

It looks like the crash is fixed now, thank you. I'd be curious on why the diagnostic request is trying to happen on a missing document ID (is there a memory leak somewhere?), but a little defensive programming didn't hurt anyone.

I've been testing this PR against v4.10.0 and so far it's been working (unlike the master branch, since eslint needs this now I think), although I am still getting the issue where it doesn't pick up the first open file. With my testing just now, it seems that it also doesn't pick up if you open helix blank and then open a file via the picker, you then have to re-open the file (or make an edit) before it starts showing anything. IMO I think functionally it's fine for this PR, but it may be worth opening an issue afterwards as a follow-up to at least document that this is known dodgey behaviour.

[SofusA]

I looked into it, and i am not sending the pull diagnostics request, because of a race between the event, and the launching the language server (and registering capabilities). The event wins 😄
In sequent events (open or edit) the language server will be initiated and capabilities are registered.

I have not found a good solution for this, which i find very annoying.
I have found a somewhat hacky solution, where i dispatch the DocumentDidOpen again after a language sever is initialized.

It looks like the crash is fixed now, thank you. I'd be curious on why the diagnostic request is trying to happen on a missing document ID (is there a memory leak somewhere?), but a little defensive programming didn't hurt anyone.

I am not completely sure why. I think part of it was because i was doing this asynchronous, where i collected all document_ids in a hashset, and then send all notifications after a debounce. I changed this to a synchronous hook instead.

[SofusA]

I have tried to address comments in separate commits.
Some changes has made this pull request large. I am sorry about that 😄

let me know if it needs some refactoring