Skip to content

Fix handling of unchanged diagnostics report #3012

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.

Sign up for GitHub

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

Open
jwortmann wants to merge 3 commits into
base: main
Choose a base branch
from
+23 −15
Open

Fix handling of unchanged diagnostics report #3012

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d309985
Fix handling of unchanged diagnostics report
jwortmann Aug 15, 2026
f3d3277
Handle WorkspaceUnchangedDocumentDiagnosticReport too
jwortmann Aug 15, 2026
fed82d0
Move comment and improve wording
jwortmann Aug 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions plugin/core/sessions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2059,16 +2059,16 @@ def _on_workspace_diagnostics_async(
) -> None:
if reset_pending_response:
self.workspace_diagnostics_pending_responses[identifier] = None
for diagnostic_report in response['items']:
uri = normalize_uri(diagnostic_report['uri'])
version = diagnostic_report['version']
for report in response['items']:
uri = normalize_uri(report['uri'])
version = report['version']
# Skip if outdated
if isinstance(version, int) and (session_buffer := self.get_session_buffer_for_uri_async(uri)) and \
version < session_buffer.last_synced_version:
continue
self.diagnostics_result_ids[(uri, identifier)] = diagnostic_report.get('resultId')
if is_workspace_full_document_diagnostic_report(diagnostic_report):
self.handle_diagnostics_async(uri, identifier, version, diagnostic_report['items'])
self.diagnostics_result_ids[(uri, identifier)] = report.get('resultId')
diagnostics = report['items'] if is_workspace_full_document_diagnostic_report(report) else None
self.handle_diagnostics_async(uri, identifier, version, diagnostics)

def _on_workspace_diagnostics_error_async(self, identifier: DiagnosticsIdentifier, error: ResponseError) -> None:
if error['code'] == LSPErrorCodes.ServerCancelled:
Expand Down Expand Up @@ -2209,7 +2209,11 @@ def on_text_document_publish_diagnostics(self, params: PublishDiagnosticsParams)
self.handle_diagnostics_async(params['uri'], None, None, params['diagnostics'])

def handle_diagnostics_async(
self, uri: DocumentUri, identifier: DiagnosticsIdentifier, version: int | None, diagnostics: list[Diagnostic]
self,
uri: DocumentUri,
identifier: DiagnosticsIdentifier,
version: int | None,
diagnostics: list[Diagnostic] | None
) -> None:
mgr = self.manager()
if not mgr:
Expand All @@ -2218,8 +2222,11 @@ def handle_diagnostics_async(
if isinstance(reason, str):
debug("ignoring unsuitable diagnostics for", uri, "reason:", reason)
return
self.diagnostics.set_diagnostics(uri, identifier, diagnostics)
mgr.on_diagnostics_updated()
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()
Comment on lines +2225 to +2227

@rchl rchl Aug 19, 2026 •
edited
Loading

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?

@jwortmann jwortmann Aug 19, 2026

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?

@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 think it would.

@jwortmann jwortmann Aug 19, 2026

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.

if session_buffer := self.get_session_buffer_for_uri_async(uri):
self._publish_diagnostics_to_session_buffer_async(
session_buffer, self.diagnostics.get_diagnostics_for_uri(uri), version)
Expand Down
12 changes: 6 additions & 6 deletions plugin/session_buffer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,14 +671,14 @@ def _on_document_diagnostic_async(
self._diagnostics_versions[identifier] = version
self._document_diagnostic_pending_requests[identifier] = None
self.session.diagnostics_result_ids[(self._last_known_uri, identifier)] = response.get('resultId')
if is_related_full_document_diagnostic_report(response):
self.session.handle_diagnostics_async(self._last_known_uri, identifier, version, response['items'])
diagnostics = response['items'] if is_related_full_document_diagnostic_report(response) else None
self.session.handle_diagnostics_async(self._last_known_uri, identifier, version, diagnostics)
if related_documents := response.get('relatedDocuments'):
for uri, diagnostic_report in related_documents.items():
for uri, report in related_documents.items():
uri = normalize_uri(uri)
self.session.diagnostics_result_ids[(uri, identifier)] = diagnostic_report.get('resultId')
if is_full_document_diagnostic_report(diagnostic_report):
self.session.handle_diagnostics_async(uri, identifier, None, diagnostic_report['items'])
self.session.diagnostics_result_ids[(uri, identifier)] = report.get('resultId')
diagnostics = report['items'] if is_full_document_diagnostic_report(report) else None
self.session.handle_diagnostics_async(uri, identifier, None, diagnostics)

def _on_document_diagnostic_error_async(
self, view: sublime.View, identifier: DiagnosticsIdentifier, version: int, error: ResponseError
Expand Down