-
Notifications
You must be signed in to change notification settings - Fork 183
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 issues with diagnostics panel toggling on save #2063
Conversation
For the sake of reviewers decided to revert introduction of |
Tested this. Works fine if there is one session that publishes diagnostics. ✔️ Doesn't work if there are 2 sessions that publish diagnostics. (for example LSP-pyright and diagnostics-ls). ⚔️ I noticed that if there are multiple sessions in the view, nor will the panel close if there are no diagnostics: Diagnostics-ls configuration{
"clients": {
"diagnostic-ls": {
"enabled": true,
"command": [
"diagnostic-languageserver",
"--stdio",
"--log-level",
"2"
],
"selector": "source.python",
"initializationOptions": {
"linters": {
"cspell": {
"command": "cspell",
"debounce": 100,
"args": [
"%filepath"
],
"sourceName": "cspell",
"formatLines": 1,
"formatPattern": [
// the error message looks like this:
// /home/predrag/.config/sublime-text/Packages/Sone/index.py:1:1 - Unknown word (funer)
".*?:(\\d+):(\\d+)\\s*-\\s*(.*)",
{
"line": 1,
"column": 2,
"message": 3,
}
],
},
// "flake8": {
// "command": "flake8",
// "debounce": 100,
// "args": [
// "--format",
// "%(row)d,%(col)d,%(code).1s,%(text)s [%(code)s]",
// "-"
// ],
// "sourceName": "flake8",
// "formatLines": 1,
// "formatPattern": [
// "(\\d+),(\\d+),()([A-Z]),(.*)",
// {
// "line": 1,
// "column": 2,
// "security": 3, // Match a zero-length group so everything can be classified as a Warning rather than Error
// "message": 5,
// }
// ],
// "securities": {
// "": "warning",
// },
// },
},
"formatters": {},
"filetypes": {
// "python": "cspell",
// "python": "cspell",
"python": "flake8",
},
},
},
}
} |
I have been testing with 2 active sessions (pyright and pylsp). Maybe there is something special about |
Here are the logs when:
Here are the logs when:
apparetnly diagnostic-ls never publishes the diagnostics. |
I think I have misunderstood a bit how diagnostics version works. Server can report outdated diagnostics for given version and then update them later for the same version. That messes up with the logic I've applied here... It's then pretty hard to fix reliably so that the panel only opens/closes after save rather than whenever diagnostics change. |
* main: Ensure "Source Actions..." request includes the "source" kind (#2064)
Implemented new approach that should work better. As long as view's |
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.
I am pretty happy with how this PR ended up.
Hopefully now the all issues regarding diagnostic panel on save are fixed with this PR.
I am just looking at a way if we can remove some code,
but we can do that later if necessary.
* main: Fix issues with diagnostics panel toggling on save (#2063) Request color presentations when clicking on a color box (#2065) Import new imports like NotRequired from "typing" Ensure "Source Actions..." request includes the "source" kind (#2064) refactor(types): Mark properties with NotRequired (#2062) Add template variable `$text_document_position` in execute command (#2061) Only update content of diagnostics panel when visible (#2054) Remove Range class and rename RangeLsp to Range (#2058)
Showing or hiding diagnostics panel on save was generally broken.
Changes:
The code for showing was in
SessionBuffer
and the code for hiding was inDocumentSyncListener
. Now everything is inDocumentSyncListener
.Removed
AbstractViewListener.diagnostics_async
interface as it didn't need to be public (not currently used by anything outside ofDocumentSyncListener
).DiagnosticsManager
class renamed toDiagnosticsStorage
since it wasn't really a manager but just glorified session diagnostics storage.Created actualREVERTEDDiagnosticsManager
that is an per-window interface to access diagnostics for all sessions. It's not essential currently but I think it's cleaner to have it and we could move more stuff there in the future.SessionBuffer.total_errors
andSessionBuffer.total_warnings
removed as those were unused.Also drive-by fix to fix regression introduced by Only update content of diagnostics panel when visible #2054 where
show_diagnostics_count_in_view_status
stopped working since the code to update the status was running fromupdate_diagnostics_panel_async
which was no longer triggered.