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

server: verification operates on file contents on disk, not in editor #160

Open
samcowger opened this issue Feb 19, 2025 · 5 comments
Open

Comments

@samcowger
Copy link
Contributor

@cpitclaudel reports that verification operates on whatever version of a file is saved to disk, rather than whatever appears in the editor. These might differ if e.g. a user tries to verify a file with unsaved changes.

For the time being, the easiest way to avoid this, as a user, is to always save changes before kicking off verification. This doesn't seem like the neatest solution, but it may match other language's existing IDE integrations. If I open a Rust file in VSCode and introduce an error, I see that rust-analyzer (which I consider an exemplary language integration) does not publish that error's diagnostics until and unless I save the file.

In the longer term, I think we can probably do better than this, though. I can think of a few possible approaches, roughly ordered by least to most principled:

  • The client can automatically save the file whenever a user requests verification before passing that request on to the server
  • The client can send all (or perhaps part) of the in-editor version of the file to the server whenever a user requests verification, or the server can request it
  • The server can keep track of a user's changes to a file (LSP explicitly supports this sort of tracking with TextDocumentSyncKind.Incremental-type changes, and I think (but am not sure) that more-sophisticated language servers tend to do this)
@cpitclaudel
Copy link
Collaborator

The server can keep track of a user's changes to a file (LSP explicitly supports this sort of tracking with TextDocumentSyncKind.Incremental-type changes, and I think (but am not sure) that more-sophisticated language servers tend to do this)

Almost all LSP servers that I use do this (they need it for semantic highlighting, go to definition, etc).

I think a very easy fix here would be to make "verify with CN" save the file.

@bcpierce00
Copy link
Collaborator

It looks like at the moment the "Verify" button is only visible when the file is saved. I found this quite puzzling at first, and even now that I'm used to it I still find it annoying. For the moment, could we rename the button "Save and verify" and have it do both automatically?

@bcpierce00
Copy link
Collaborator

(Oh, I see that Clement already made this suggestion!)

@samcowger
Copy link
Contributor Author

I agree that this hide-buttons-on-change behavior is kinda gross, but it is a very temporary hack. Once the server is set up to track document changes, it should be able to continuously republish them in the right place, avoiding the behavior described in #158. That issue is the main reason for only enabling the buttons on saved documents in the first place.

The other reason is more technical. Button payloads include arguments that are computed based on the document the server has access to when the buttons are created. As this issue describes, this is always the on-disk version. If a button click triggered a document save, updating the document on disk, then the arguments would immediately go out of sync with the document. The buttons would need to be recreated to bring them up to date, but it would be too late, since one was just clicked. This, too, will be fixed by having the server track the in-buffer version of the document, since it will be continuously recomputing buttons based on whatever version of the document a user sees.

@bcpierce00
Copy link
Collaborator

Thanks for the background. (And wow, that's pretty disgusting.) The present behavior is fine for the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants