Skip to content

Fix handling of unchanged diagnostics report - #3012

Open
jwortmann wants to merge 3 commits into
sublimelsp:mainfrom
jwortmann:fix-handling-unchanged-diagnostics
Open

Fix handling of unchanged diagnostics report#3012
jwortmann wants to merge 3 commits into
sublimelsp:mainfrom
jwortmann:fix-handling-unchanged-diagnostics

Conversation

@jwortmann

Copy link
Copy Markdown
Member

I noticed that UnchangedDocumentDiagnosticReport is not handled correctly. Unchanged diagnostics report means that the previous diagnostics (and as far as I understand, including the diagnostic's range) are still accurate. Currently we do "nothing" when we receive such a report:

if is_related_full_document_diagnostic_report(response):
self.session.handle_diagnostics_async(self._last_known_uri, identifier, version, response['items'])
if related_documents := response.get('relatedDocuments'):

But actually we need to redraw diagnostics regions in the view, because diagnostics are typically requested after a buffer change, and ST automatically adjusts/shifts existing region decorations if lines before the region we removed or added, or characters were inserted or removed on the same line before the region.

If lines or characters were inserted or removed after the diagnostics regions it didn't matter, because in that case the drawn regions are not affected anyway.

I added another None value as possible argument to Session.handle_diagnostics_async, which means that we received unchanged diagnostics, i.e. stored diagnostics don't need to be updated, but we still need to trigger a redraw.

Comment thread plugin/core/sessions.py
Comment on lines +2225 to +2229
if diagnostics is not None:
# `None` means we received an UnchangedDocumentDiagnosticReport, in which case we still have to redraw
# diagnostic regions in the view to maintain the original positions.
self.diagnostics.set_diagnostics(uri, identifier, diagnostics)
mgr.on_diagnostics_updated()

@rchl rchl Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm coming here without refreshing my memory on how diagnostics rendering works but just reading that comment makes me a bit confused because it talks about still having to handle the None case but being inside the !None branch. Is it misplaced?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah maybe it's a bit misplaced, I think I put it here because these two lines are the only part of this method which is affected by diagnostics being None or not None. And there is no else branch. I could put the comment after this if-block, if that makes it clearer?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in fed82d0 and tried to make the wording a bit more clear.

@netlify

netlify Bot commented Aug 19, 2026

Copy link
Copy Markdown

Deploy Preview for sublime-lsp ready!

Name Link
🔨 Latest commit fed82d0
🔍 Latest deploy log https://app.netlify.com/projects/sublime-lsp/deploys/6a861fda3a04c800074c4b62
😎 Deploy Preview https://deploy-preview-3012--sublime-lsp.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

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

Successfully merging this pull request may close these issues.

2 participants