Skip to content

Comments

[ty] Add capabilities check for clear_diagnostics#20989

Merged
MichaReiser merged 1 commit intoastral-sh:mainfrom
TaKO8Ki:capabilities-check-for-clear_diagnostics
Oct 20, 2025
Merged

[ty] Add capabilities check for clear_diagnostics#20989
MichaReiser merged 1 commit intoastral-sh:mainfrom
TaKO8Ki:capabilities-check-for-clear_diagnostics

Conversation

@TaKO8Ki
Copy link
Contributor

@TaKO8Ki TaKO8Ki commented Oct 20, 2025

Summary

Fixes #20784

Test Plan

I have tested it with VSCode.

output

@sharkdp sharkdp removed their request for review October 20, 2025 11:09
Comment on lines +124 to +126
if session.client_capabilities().supports_pull_diagnostics() {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same handling as:

if session.client_capabilities().supports_pull_diagnostics() {
return;
}

@MichaReiser MichaReiser added server Related to the LSP server ty Multi-file analysis & type inference labels Oct 20, 2025
@MichaReiser
Copy link
Member

Would you mind testing this change with VS code? Specifically, that the diagnostics "disappear" after closing the corresponding file.

@TaKO8Ki TaKO8Ki force-pushed the capabilities-check-for-clear_diagnostics branch from de9ecd9 to f0f51e7 Compare October 20, 2025 13:49
@TaKO8Ki
Copy link
Contributor Author

TaKO8Ki commented Oct 20, 2025

@MichaReiser I have confirmed it and uploaded a GIF.

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Perfect, thank you

@MichaReiser MichaReiser enabled auto-merge (squash) October 20, 2025 13:58
@TaKO8Ki
Copy link
Contributor Author

TaKO8Ki commented Oct 20, 2025

Are the self hosted runners unstable now?

@MichaReiser
Copy link
Member

Yeah, they're affected by the AWS outage https://status.depot.dev/

auto-merge was automatically disabled October 20, 2025 17:20

Head branch was pushed to by a user without write access

@TaKO8Ki TaKO8Ki force-pushed the capabilities-check-for-clear_diagnostics branch from f0f51e7 to 7ac999a Compare October 20, 2025 17:20
@github-actions
Copy link
Contributor

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@github-actions
Copy link
Contributor

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

@MichaReiser MichaReiser merged commit 1197035 into astral-sh:main Oct 20, 2025
39 of 40 checks passed
@TaKO8Ki TaKO8Ki deleted the capabilities-check-for-clear_diagnostics branch October 20, 2025 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clear_diagnostics sends a publishDiagnostics notification without checking the client capabilities

2 participants