Add pullDiagnostics lsp feature

[Stanislav-Lapata]

trafficstars

Some of LSP support only pull diagnostics source here https://github.com/helix-editor/helix/discussions/7725

[woojiq]

We need to wait until https://github.com/gluon-lang/lsp-types/pull/258 is merged. Right now library has no primitives to implement textDocument/diagnostic.

[pascalkuthe]

The new lsp types version contains the necessary PR so this is not blocked anymore. It might be a bit of work to implement tougnh

[woojiq]

Are there any other language servers that you know of that support pull diagnostics? I've tried omnisharp, pyright, RA, zls, and a few others, but none have implemented this feature. Some of them will probably support this later because they also depend on lsp-types as nil. So it will even be hard to test decently with different ls 😄

[pascalkuthe]

This issue was motivated by ruby-lsp. It's a fairly new feature tough so I am not surprised it's not widely supported yet

[woojiq]

I played around with the implementation a bit and here are some notes. It's actually very easy to implement if you make this request on a key press (like hover or goto-definition). This is because all such commands are executed with a commands::Context passed in, so we can use cx.callback, which gives as mut Editor in the closure, and we can extract documents and other stuff: https://github.com/helix-editor/helix/blob/57f093d83641642ad5d4ba42ae59f03272efcfcc/helix-term/src/commands/lsp.rs#L1291-L1293

This is how all commands interact with lsp (via future + callback). On the other hand, the current diagnosis (publish) comes to us in the form of notifications, so we no longer need to interact with async, just change the document and listen for additional notifications. So my question is how can we simultaneously process an asynchronous request and then make changes to the document doc.replace_diagnostics(). For example, we can't paste code here: https://github.com/helix-editor/helix/blob/57f093d83641642ad5d4ba42ae59f03272efcfcc/helix-view/src/document.rs#L915-L919

because mut Doc cannot be passed between threads. By code I mean lsp call + await + handle the returned data. Do we need to refactor the code and implement a callback? I hope you understand what I mean. :crossed_fingers::smiley:

P.S. if you also want to try to implement this feature, ruby-lsp works with pullDiagnostic, but you also need to download robocup and look carefully at the docs. This lsp works very limited compared to RA.

[pascalkuthe]

We have mechanismis for exactly this: You need to create a job. This gets run in the main loop mutable access to the editor. This should likely run on idle timeout (and need to use the event system in the future)

[SofusA]

What would be required now that #8021 is merged?

I could give this a shot, if it is not too difficult.

I would like to try out Roslyn Language Server (successor to OmniSharp) for C# and it only supports pull diagnostics.

[woojiq]

I could give this a shot, if it is not too difficult.

I think that would be not very difficult, because all building blocks are probably in the code base already. But you need to carefully read and understand lsp specification to support ?all? capabilities, etc.

[gobijan]

This is very much needed to work with Ruby-LSP the defacto LSP for Ruby and Ruby on Rails. Any update on this? I don't speak Rust yet but am willing to support / sponsor the development of this feature.