Stream: t-compiler/rust-analyzer

Topic: Dynamically moving flycheck diagnostics


Jonas Schievink [he/him] (Dec 07 2020 at 13:30, on Zulip):

I was looking into automatically moving flycheck diagnostics around when the containing file is edited, but this seems like a fairly nontrivial algorithm to implement. Is there an existing implementation or algorithm description of something like this that I could look at?

matklad (Dec 07 2020 at 13:31, on Zulip):

I feel pretty strongly that this should be handled by the editor

matklad (Dec 07 2020 at 13:32, on Zulip):

It can adjust ranges synchronously, and it doesn't need any langauge-specific info for that.

I wonder if non-VS clients do this?

Jonas Schievink [he/him] (Dec 07 2020 at 13:33, on Zulip):

Yeah, but I don't think any of them actually do this

Jonas Schievink [he/him] (Dec 07 2020 at 13:33, on Zulip):

VS Code also doesn't

matklad (Dec 07 2020 at 13:36, on Zulip):

Yeah... I think this is the case where it's better to not solve a problem at all, than to solve it in a wrong way.

matklad (Dec 07 2020 at 13:36, on Zulip):

By making range updating "async" we kind of loose any hope to have consistent results

matklad (Dec 07 2020 at 13:37, on Zulip):

Clients, on the other hand, can adjust ranges in lockstep with updating the text buffer (whcih they do anyway for, eg, syntax highlighting)

matklad (Dec 07 2020 at 13:39, on Zulip):

that said, synchronization concerns aside, this shouldn't be hard at the core

matklad (Dec 07 2020 at 13:39, on Zulip):

See TextEdit::apply_to_offset

matklad (Dec 07 2020 at 13:40, on Zulip):

(plus, there should be additional handling for "sticky" span edges. For highlighting, you, eg, want to extend the span of idenifier if a user types next to it)

Jeremy Kolb (Dec 07 2020 at 13:43, on Zulip):

I remember seeing something about moving to a pull model for diagnostics. If a server switched to that it might be easier to get the right logic into the client

matklad (Dec 07 2020 at 13:47, on Zulip):

riiiight, there's also protocol-level concern that both edits and diagnostics are notifications (going in the opposite directions), so it's actually impossible to order them

matklad (Dec 07 2020 at 13:47, on Zulip):

(although document version might be enough for this particular case of shifting edits)

Florian Diebold (Dec 07 2020 at 13:56, on Zulip):

hm. can the editor do this for the check-based diagnostics though? just from how emacs behaves, it looks to me like we are sending the diagnostics with the wrong range if the user edits without saving (i.e. we're sending the old check diagnostics with the now-stale ranges again?)

matklad (Dec 07 2020 at 13:59, on Zulip):

Hm, I think that would be a different problem? Basically, we compute just the wrong diagnostics in this case (not only the ranges), and we should force client to save documents when computing diagnostics

Florian Diebold (Dec 07 2020 at 14:00, on Zulip):

well I mean our internal diagnostics are correct and move correctly, because they use the unsaved document

Jonas Schievink [he/him] (Dec 07 2020 at 14:00, on Zulip):

hmm, and if we publish internal diagnostics only, that would override the check diagnostics

Florian Diebold (Dec 07 2020 at 14:01, on Zulip):

Emacs doesn't move diagnostics at all by the way, it just hides all diagnostics as soon as you type and displays the new ones once they arrive

Jeremy Kolb (Dec 07 2020 at 14:07, on Zulip):

LSP issue for pull model: https://github.com/microsoft/language-server-protocol/issues/737. It looks like this is an issue about interactive diagnostics for vscode: https://github.com/microsoft/vscode/issues/103953

Jonas Schievink [he/him] (Dec 07 2020 at 14:10, on Zulip):

Isn't the push model much better suited for... everything? If the client has to pull it might not update when eg. an import now resolved because a different file changed

Jonas Schievink [he/him] (Dec 07 2020 at 14:10, on Zulip):

This is also a problem for semantic tokens, I think

matklad (Dec 07 2020 at 14:11, on Zulip):

@Jonas Schievink I still believe that the right model here is that of dart analysis server

matklad (Dec 07 2020 at 14:11, on Zulip):

https://htmlpreview.github.io/?https://github.com/dart-lang/sdk/blob/master/pkg/analysis_server/doc/api.html#request_analysis.setSubscriptions

Jonas Schievink [he/him] (Dec 07 2020 at 14:11, on Zulip):

push + subscriptions? yeah

matklad (Dec 07 2020 at 14:11, on Zulip):

yup, that one

matklad (Dec 07 2020 at 14:12, on Zulip):

And also requiring replies for changes notification, so that the client knows what state of the world the serve sees

Jeremy Kolb (Dec 07 2020 at 14:25, on Zulip):

Does the dart LSP stack use the subscription model? I've used their stuff on vscode for some flutter work and it's pretty slick.

matklad (Dec 07 2020 at 14:26, on Zulip):

I don't know

Last update: Jul 29 2021 at 09:45UTC