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

Implement the new LSP diagnostic pull model #3600

Open
w0rp opened this issue Feb 20, 2021 · 5 comments
Open

Implement the new LSP diagnostic pull model #3600

w0rp opened this issue Feb 20, 2021 · 5 comments
Assignees
Labels
enhancement LSP Any issue relating to LSP or tsserver
Milestone

Comments

@w0rp
Copy link
Member

w0rp commented Feb 20, 2021

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

microsoft/vscode#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!)
@w0rp w0rp added enhancement LSP Any issue relating to LSP or tsserver labels Feb 20, 2021
@w0rp w0rp self-assigned this Feb 20, 2021
@hsanson
Copy link
Contributor

hsanson commented Oct 9, 2021

@w0rp
Copy link
Member Author

w0rp commented May 11, 2022

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

@w0rp w0rp changed the title Implement the new LSP diagnostic pull model (Not available yet) Implement the new LSP diagnostic pull model May 12, 2022
@w0rp
Copy link
Member Author

w0rp commented May 12, 2022

@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 w0rp pinned this issue Dec 20, 2022
@w0rp
Copy link
Member Author

w0rp commented Dec 20, 2022

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.

@towerpark
Copy link

There's probably an actual server that implements the pull model now I can test against.

rust-analyzer has just implemented the pull model, so maybe it could be used for testing if you're going to work on this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement LSP Any issue relating to LSP or tsserver
Projects
None yet
Development

No branches or pull requests

3 participants