-
Notifications
You must be signed in to change notification settings - Fork 824
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
How to have a diagnostic WITHOUT associated file? #256
Comments
Fully agree. From the alternatives I prefer having the workspace root as the URI for these errors. Additional question is the range in this situation. That should be omittable. @sandy081 can we add some code to the problems view to check if the URI is a file and if not then we don't try to open the editor ? |
@dbaeumer Not sure if it is right approach. Because only opening file based resources might break other scheme resources which support opening. |
Any update? |
No, not yet. But thanks for pinging. @sandy081 I am not sure I understand your concerns. The schema would be 'file' as well. When the user clicks on it in the problems view we would reveal it in the folder instead of opening the editor. |
@dbaeumer If I understand correctly, you do not want to open resources from the problems view? I think that will not be helpful, because most users prefer to open in the editor directly. |
@sandy081 I was not very clear. The resource would be a file resource, but pointing to a folder (the workspace root) not an actual file. Hence it can't be opened in the editor, but revealed in the navigator or we can even decide to do nothing. |
@dbaeumer I see.. I was confused with file scheme resource vs other scheme resources. Have to check with Ben if I can have such a check (api). |
@sandy081 thanks. |
Currently, opening a folder URI from Problems panel, reveals the folder in Explorer. |
@sandy081 thanks for clarifying. |
We currently have the same problem over in or LSP for Nim. It's more related to ranges, but as #249 was marked as a duplicate of this I'll comment here. So what is the right way to send an error, hint, or warning that's not related to a specific position in a document, or to no specific document at all? I think this should be a new type, instead of overloading the Diagnostics type with more optional fields (e.g. what does a range mean if you don't supply a uri?). So create a new type that just contains information from the compiler, and keep the Diagnostics type as it is today to show messages belonging to a piece of code. |
hello, any update? |
Summary: My goal is to report hh_server status in a diagnostic, not in the status bar. This will squarely address complaints of users who say "I never realized that hh_server wasn't reporting errors". It will also make the status bar cleaner. VSCode and LSP don't support "general" diagnostics. They only support diagnostics attached to a real file: microsoft/language-server-protocol#256 I therefore decided that I will attach the status-diagnostic about "hh_server not running" to the most recently used file, if it's open. This diff is in service of that goal. It simply tracks which was the file most recently used. There's no guarantee that the file is still open or anything. It just tracks the most recent file. Reviewed By: arxanas Differential Revision: D20876768 fbshipit-source-id: 7328c163212fd9d2309480d25713d768af6cf11a
I'm interested in this as well, any update? |
Have you tried using a folder URI? If it is a project configuration error the URI could be the project file or if nothing else can be used the workspace folder URI. |
You can use the folder URI, but IIRC then the diagnostic stays there forever, the editor never refreshes it. |
@alessiostalla Which LSP client are you using? This sounds like something that might be client-specific. |
I'm using Visual Studio Code. |
Is that allowed by the protocol ? AFAICT the 'publishDiagnostics' notification takes a Certainly my client has to have a "catch" on this because some servers like jdt.ls return URIs which are not files, which i consider to be noncompliant. |
@puremourning Good point, I had forgotten about that detail.
Not too surprised by that given how Eclipse's Java compiler does things. |
Good points. But you can always point to a project file. But I still think we could relax this a little and allow folder URIs as well. |
I've ran into this issue as well. Have the uri optional or pointing it to the workspace root dir would both be adequate. |
Imagine you compile a C# program without a 'main' entrypoint, and it generates the error message
It's impossible to report this error via LSP! That's because the
publishDiagnostics
message has a mandatoryuri
field -- but for errors like this, there's nouri
that would make sense! And so the C# plugin for VSCode shows this message in the build output window, but simply doesn't report this error at all to VSCode diagnostics list. @DustinCampbellI experimented with VSCode behavior:
publishDiagnostics with uri = null
-- this doesn't get shown in the diagnostics window at allpublishDiagnostics with uri = ""
-- this sometimes pops up the Chrome debugger in VSCode, and sometimes crashes VSCode completely with a "report this crash to Apple" system dialog.publishDiagnostics with uri = "file:///"
oruri = "file:///some/directory
-- this shows the error in the diagnostics window, and when you click on it, VSCode does some frenetic flickering of a new document tab before closing it again.I think that "diagnostics not related to a particular file" are a real fact of life. LSP should be extended to cope with them. And VSCode should too.
My proposal: make the
uri
field optional. If it is absent/null, then treat it as a file-less diagnostic, and therange
field inside each individual Diagnostic will be ignored.The text was updated successfully, but these errors were encountered: