search

dense-analysis / ale

Check syntax in Vim/Neovim asynchronously and fix files, with Language Server Protocol (LSP) support
BSD 2-Clause "Simplified" License
13.49k stars 1.43k forks source link

Implement the new LSP diagnostic pull model #3600

Open w0rp opened 3 years ago

w0rp commented 3 years ago

What is the bug?

  1. You do just about anything with a language server, such as completion
  2. You have to send the document updates to the language server
  3. Without the "pull model" the client (ALE) has no control over when diagnostics are sent
  4. You can't just "ignore" diagnostics "until you are ready," you have to show them when they come
  5. Users perceive that "linting" is happening when they don't want it to, which annoys users

We fix it by implementing the pull model and forcing servers to adapt, plus being careful about when we send document updates.

History

https://github.com/microsoft/vscode/issues/117042

A proposal under review will allow LSP clients to explicitly request diagnostics from language servers, instead of receiving them at random. This will fix issues such as #3555 where you can receive diagnostics due to ALE having to tell servers that changes have been made to files for features like completion to work, even though you don't want them yet.

If/when this proposal is adopted and the protocol is updated, we should implement support for it in ALE so newer versions of servers automatically stop causing this problem for users over time.

Update August 2024: This became part of the Language Server Protocol standard, and now depending on the language server, we can support this in ALE.

Implementation

  1. Implement the pull model!
  2. Limit the number of document updates based on updates that are strictly necessary for functionality only. (Hard to manage!)
hsanson commented 2 years ago

Still proof of concept: https://github.com/microsoft/vscode-languageserver-node/blob/main/protocol/src/common/proposed.diagnostics.md#L1

w0rp commented 2 years ago

Someone nudge me when this is implemented in the actual spec, and I'll support it.

w0rp commented 2 years ago

@hsanson They actually did it in the past couple of days! The suggestion I made for a "pull" model for diagnostics instead of a "push" model as an option has been written into the protocol spec. Now we can implement support for it in ALE as the preferred option, so you only see diagnostics in response to an ALE lint cycle.

w0rp commented 1 year ago

There's probably an actual server that implements the pull model now I can test against. I pinned this issue so I can try to remember to implement this.