-
Notifications
You must be signed in to change notification settings - Fork 26
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
Clear diagnostic results on documents #123
Conversation
When a document is scanned and no errors are returned, no diagnostic is reported to the LSP_SERVER. This change checks for diagnostics against the specific file. If none are created, it clears the errors on that file by reporting an empty set of diagnostics. Fix for: microsoft#119
@karthiknadig would appreciate a look over if you can? more details on the bug |
@samskiter Thanks for looking into this issue. I have a few questions. I am actually thinking of actually switching this to use a different request: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_pullDiagnostics For debugging you can actually use the test view to debug specific tests: |
@karthiknadig Agree that the architecture doesn't quite look right for workspace reporting setting - as you point out, this fix would be unable to clear documents that aren't requested directly from the LSP (i.e. those from included documents, the mypy response doesn't include a list of files it scanned it seems) I believe that the proposed fix would at least remove the most annoying/frustrating aspects of this bug - that you can edit a file and have it go from Red to Green without having to restart the mypy-type-checker server (which is currently in my muscle memory!). It should be fine for the file-level reporting and the workspace reporting scope is still market experimental by you right? So this might be a good stop-gap while you undertake a bigger refactor as you mention (should probably be tracked in a separate ticket, IMO)? WDYT? I'm going to also have a quick look to see if there's a way to get mypy to give a response for every file it inspected... might make this even easier to fix up? |
I agree with this as a stop gap fix. Created #124 to track the bigger change. |
Also raised: python/mypy#15902 which might make implementing this a little easier |
eeede0f
@microsoft-github-policy-service agree |
Need a Bug label adding please @karthiknadig |
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.
Lint is failing.
@karrtikr fixed :) |
Seems to be still failing.. Did you forget to commit all changes by chance? |
i formatted the file with black - which is what I thought was failing |
ah, there was a second file that needed formatting. pushed |
OK think it's good to go now @anthonykim1 |
plz can haz review? |
Just need 1 more review and then we can merge? |
Thanks all! 🥳 |
When a document is scanned and no errors are returned, no diagnostic is reported to the LSP_SERVER. This change checks for diagnostics against the specific file. If none are created, it clears the errors on that file by reporting an empty set of diagnostics.
Tested with unit tests, but not installed and tested yet...
Fixes: #119