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

Initialize diagnostics when opening a document #8873

Merged

Conversation

Philipp-M
Copy link
Contributor

I've noticed, that diagnostics that are sent from the language server aren't propagated when opening a document (via helix_view::Document::open). So that the language server needs to send a PublishDiagnostics again to populate the document with diagnostics. This can take some time (noticed with rust-analyzer), when opening a file via the workspace diagnostic picker (if it is sent at all).

This PR should ensure that diagnostics that are already in the editor instance are propagated when opening a document.
This has the additional advantage, that diagnostics can be rendered directly in the workspace diagnostic picker, when the file/buffer isn't open yet.

helix-view/src/document.rs Outdated Show resolved Hide resolved
@the-mikedavis the-mikedavis added A-language-server Area: Language server client S-waiting-on-review Status: Awaiting review from a maintainer. labels Nov 21, 2023
helix-view/src/document.rs Outdated Show resolved Hide resolved
helix-view/src/document.rs Outdated Show resolved Hide resolved
@Philipp-M Philipp-M force-pushed the diagnostics-on-document-open branch 2 times, most recently from cc392c2 to 83d85cb Compare December 18, 2023 14:44
@Philipp-M
Copy link
Contributor Author

Highlights in workspace diagnostics picker didn't work anymore with the newest changes, the last commit should fix that

@Philipp-M
Copy link
Contributor Author

Since there were quite some merge conflicts to resolve, I tried again with reusing Editor::doc_diagnostics (by not acting directly on the editor reference, but instead the registry and diagnostics), so in that form it probably improves it.

Can you check again @pascalkuthe?

Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good to me now! Mostly just some style nits. I will test this locally for a few days.

One thing that is still missing is that you need to repopulate diagnostics on config reload in case the diagnostics feature was enabled/disabled for some LS.

helix-term/src/application.rs Outdated Show resolved Hide resolved
@Philipp-M
Copy link
Contributor Author

One thing that is still missing is that you need to repopulate diagnostics on config reload in case the diagnostics feature was enabled/disabled for some LS.

I think I have covered this now everywhere where the language configuration can be changed, let me know if I missed something (or was to thorough).

@archseer archseer merged commit 41ca46c into helix-editor:master Jan 9, 2024
6 checks passed
@Philipp-M Philipp-M deleted the diagnostics-on-document-open branch January 9, 2024 01:24
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants