-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Don't send publish diagnostics for clients supporting pull diagnostics #21772
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
[ty] Don't send publish diagnostics for clients supporting pull diagnostics #21772
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
5015526 to
3abadc2
Compare
3abadc2 to
1e71d29
Compare
Yeah, I'm not sure either. I looked at the request payload: It seems to be sending an entry stating that the |
Summary
fixes: astral-sh/ty#1727
This PR fixes the reported duplicated diagnostic in astral-sh/ty#1727 by skipping
publish_diagnosticsif pull diagnostics are enabled, similar to how we do this in other places.
While this PR fixes the duplicate diagnostics, there's still the question why vim sends
didChangeWatchedFilesfora file that's currently open in the editor. The issue with that is that the server is now in this awkward spot
that it has to decide whether it should use the new content on disk OR the content that's currently open in the editor (as reported by
didOpen,didChange). For now, ty uses the content sent bydidOpen/didChange,meaning, ty will ignore the new content when checking the project.
I think this is the sensible thing to do here. The client either has to tell the server that the file is now closed,
in which case ty will use the content from disk, or that the user accepted the on-disk changes over the in-editor changes
by sending a
didChangenotification.Test Plan
Added E2E test