Open ljw1004 opened 7 years ago
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.
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 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?
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.
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.
You can use the folder URI,
Is that allowed by the protocol ? AFAICT the 'publishDiagnostics' notification takes a DocumentUri
- i can't see how a folder can legitimately be described as a document. And the Diagnostic
contains a Range
which is source offsets. This also nonsensical for a folder.
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.
Is that allowed by the protocol ? AFAICT the 'publishDiagnostics' notification takes a
DocumentUri
- i can't see how a folder can legitimately be described as a document. And theDiagnostic
contains aRange
which is source offsets. This also nonsensical for a folder.
@puremourning Good point, I had forgotten about that detail.
Certainly my client has to have a "catch" on this because some servers like jdt.ls return URIs which are not files
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.