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

Should publish diagnostic information for project level #608

Closed
yaohaizh opened this issue Aug 10, 2018 · 10 comments
Closed

Should publish diagnostic information for project level #608

yaohaizh opened this issue Aug 10, 2018 · 10 comments
Assignees

Comments

@yaohaizh
Copy link
Collaborator

yaohaizh commented Aug 10, 2018

Currently, the default diagnostic information for the default project is hidden from the customer, this will cause confusion for the customer when invoke the "Java: Build work space" or other commands depend on the build status of the workspace.

microsoft/vscode-java-debug#358

More or less, we should let the user know about the error?

@fbricon What's your opinion?

@fbricon
Copy link
Collaborator

fbricon commented Aug 10, 2018

@kdvolder's issue is related to a Maven project, has nothing to do with the default project. All build errors should be reported in his case. The fact that running the build workspace command fails silently is absolutely wrong. In the server log, the build failure should at least log which errors have been encountered. I'm curious to see what kind of errors are actually silenced from the UI.

@kdvolder
Copy link
Contributor

Maybe I'm looking in the wrong place(s)? But as far as I can tell the project is fine. There is nothing in the 'problems' view, for example which is where I would expect to see something if there was a problem building something.

If this were Eclipse and not vscode, problems with maven poms are in the problems view, and also show up as some error decorations in the package or project explorer tree. In vscode, I didn't see anything like that.

@kdvolder
Copy link
Contributor

The fact that running the build workspace command fails silently is absolutely wrong.

Since I got an error popup telling me that 'build failed' I'd say you are right, it wasn't 'silent'. It looked to me like something threw an error. This error was caught by vscode-java-debug.

But as a user, I couldn't seem to find any messages about what the actual problem was. And since vscode-java also didn't pass on the details of the message, from my point of view it felt like a 'mysterious build operation' had failed.

To me there are two things that could be done to improve this situation:

  • vscode-java-debug can improve the popup and include some more details about the 'failed operation' and its specific error message.

  • vscode-java can try to surface build errors more obviously e.g. by turning into diagnostic markers via LSP protocol.

@kdvolder
Copy link
Contributor

I'm curious to see what kind of errors are actually silenced from the UI.

I can't be sure, because I couldn't find the error. But my guess is it was the typical error you get when the pom.xml has changed and m2e wants the user to do a 'update maven projects' to fix it. At least it seems consistent with the workaround where I got rid of the error by adding some whitespace to the pom.xml and then saying yes when the popup asks if it should respond to that 'build file has changed' event.

@testforstephen
Copy link
Collaborator

I got the same Build failed error after i changed my pom.xml. From the LS log, i can find the build error details message: Project configuration is not up-to-date with pom.xml. Select: Maven->Update Project... from the project context menu or use Quick Fix.;code: 0, but in VS Code, i don't see any tip about the error details.

Through a quick look at the LS source code, i found it's a bug of BuildWorkspaceHandler.

See the code https:/eclipse/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java#L104,
it only publishes source file level of build error, but eats the project level error silently.

@yaohaizh yaohaizh changed the title Should publish diagnostic information for default java project(single file) Should publish diagnostic information for project level Aug 16, 2018
@yaohaizh
Copy link
Collaborator Author

@fbricon Okay, I turns out this is a different issue from the default project diagnostic, in both cases the diagnostic information will be swallowed in the server side.

For this issue, add the related protocol issue: microsoft/language-server-protocol#256

@kdvolder
Copy link
Contributor

kdvolder commented Aug 16, 2018

If I understand correctly the related issue (microsoft/language-server-protocol#256) is about problems that don't have a uri/resource associated with them.

This error:

Project configuration is not up-to-date with pom.xml. Select: Maven->Update Project... from the project context menu or use Quick Fix.;code: 0

Seems like it would be perfectly naturally associated with the pom.xml file so its really a different case and shouldn't have to wait for protocol to support diagnostics not associated with a resource.

BTW: If you don't want that error to read like somewhat confusing to a vscode user it probably should be reworded not to talk about the Eclipse project context menu.

@snjeza snjeza self-assigned this Aug 16, 2018
@yaohaizh
Copy link
Collaborator Author

In this case, the fix/issue is with project configuration model, but not pom.xml. User doesn't necessary to make any change to fix the problems. So we cannot associated the problems with the pom.xml directly.

@kdvolder
Copy link
Contributor

kdvolder commented Aug 17, 2018

So we cannot associated the problems with the pom.xml directly.

The message does mention the pom.xml, so i.m.o that makes it rather logical to attach the error to the pom.xml, especially if the alternative is that its not visible anywhere.

I do take your point that no change to pom is needed to fix the problem though. I just don't think that matters much from a user point of view as to where you show the error. I think in general the question isn't where you fix the problem because that is allways somehwat nebulous anyway. Take for example calling a method where there is a typo in the name. The error is really going to be the same whether the typo is in the declaration or in the call-site. But the error is allways shown on the call-site even though that is not allways where you have to fix it.

So to me, the question isn't 'do you fix the error in the pom' but 'is the error related to the pom'.

User doesn't necessary to make any change to fix the problems

That's actually a very good point. It raises the question why there is even an error here at all. The tool (m2e) probably should take care of this particular 'problem' automatically rather than issue an annoying message that asks the user to do it.

@fbricon
Copy link
Collaborator

fbricon commented Aug 31, 2018

Fixed in jdt.ls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants