Skip to content
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

Open
ljw1004 opened this issue Jun 14, 2017 · 21 comments
Open

How to have a diagnostic WITHOUT associated file? #256

ljw1004 opened this issue Jun 14, 2017 · 21 comments
Labels
diagnostics feature-request Request for new features or functionality
Milestone

Comments

@ljw1004
Copy link
Contributor

ljw1004 commented Jun 14, 2017

Imagine you compile a C# program without a 'main' entrypoint, and it generates the error message

CSC : error CS5001: Program does not contain a static 'Main' method suitable for an entry point [/Users/ljw/code/c/c.csproj]

It's impossible to report this error via LSP! That's because the publishDiagnostics message has a mandatory uri field -- but for errors like this, there's no uri 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. @DustinCampbell

I experimented with VSCode behavior:

  • publishDiagnostics with uri = null -- this doesn't get shown in the diagnostics window at all
  • publishDiagnostics 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:///" or uri = "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 the range field inside each individual Diagnostic will be ignored.

@dbaeumer
Copy link
Member

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 dbaeumer added the feature-request Request for new features or functionality label Jun 15, 2017
@dbaeumer dbaeumer added this to the 4.0 milestone Jun 15, 2017
@sandy081
Copy link
Member

@dbaeumer Not sure if it is right approach. Because only opening file based resources might break other scheme resources which support opening.

@michaellzc
Copy link

Any update?

@dbaeumer
Copy link
Member

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.

@sandy081
Copy link
Member

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.

@dbaeumer
Copy link
Member

dbaeumer commented May 9, 2018

@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.

@sandy081
Copy link
Member

sandy081 commented May 9, 2018

@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).

@dbaeumer
Copy link
Member

@sandy081 thanks.

@sandy081
Copy link
Member

sandy081 commented Jun 1, 2018

Currently, opening a folder URI from Problems panel, reveals the folder in Explorer.

@dbaeumer
Copy link
Member

dbaeumer commented Jun 6, 2018

@sandy081 thanks for clarifying.

@PMunch
Copy link
Contributor

PMunch commented Nov 10, 2018

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.

@sergiuilie
Copy link
Contributor

hello, any update?

@dbaeumer dbaeumer modified the milestones: 4.0, Backlog, On Deck Oct 30, 2019
facebook-github-bot pushed a commit to facebook/hhvm that referenced this issue Apr 10, 2020
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
@alessiostalla
Copy link

I'm interested in this as well, any update?

@dbaeumer
Copy link
Member

dbaeumer commented Jul 7, 2020

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.

@alessiostalla
Copy link

You can use the folder URI, but IIRC then the diagnostic stays there forever, the editor never refreshes it.

@rcjsuen
Copy link
Contributor

rcjsuen commented Jul 7, 2020

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.

@alessiostalla
Copy link

I'm using Visual Studio Code.

@puremourning
Copy link

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.

@rcjsuen
Copy link
Contributor

rcjsuen commented Jul 7, 2020

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.

@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.

@dbaeumer
Copy link
Member

dbaeumer commented Jul 9, 2020

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.

@rgrinberg
Copy link
Contributor

I've ran into this issue as well. Have the uri optional or pointing it to the workspace root dir would both be adequate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostics feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

10 participants