-
Notifications
You must be signed in to change notification settings - Fork 92
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
Fix Python diagnostics not being cleared on document close #6141
base: 5739-fix-lsp-methods-not-searching-user-namespace
Are you sure you want to change the base?
Fix Python diagnostics not being cleared on document close #6141
Conversation
E2E Tests 🚀 |
70947af
to
3cf8840
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like a nice simplification!
# This happens when a cell's language is changed from python e.g. to raw, | ||
# see: https://github.com/posit-dev/positron/issues/4160. | ||
if params.text_document.uri.startswith(_VSCODE_NOTEBOOK_CELL_SCHEME): | ||
return _clear_diagnostics_debounced(server, params.text_document.uri) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this is reverting some of #5950?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just empirically testing, it looks like Problems
is still working as expected swapping between raw/Python/closing a notebook 💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, this clears diagnostics when any document is closed instead of just notebook cells, although I've kept the test from #5950. It should probably have been that way from the beginning (see #6141 (comment)).
|
||
with patch.object(server, "publish_diagnostics") as mock: | ||
params = DidCloseTextDocumentParams(TextDocumentIdentifier(text_document.uri)) | ||
positron_did_close_diagnostics(server, params) | ||
|
||
# Wait for the diagnostics to be published |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not need this wait anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. I checked upstream and they clear diagnostics immediately on document close. We should have always done that, but I assume not doing that was a mistake when we first set this up.
Will merge this after #6140. |
Addresses #3905.
Release Notes
New Features
Bug Fixes
QA Notes
See the linked issue.