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

No need to publish diagnostics in BuildWorkspaceHandler #1282

Merged
merged 1 commit into from
Nov 25, 2019

Conversation

testforstephen
Copy link
Contributor

Signed-off-by: Jinbo Wang [email protected]

WorkspaceDiagnosticsHandler.java is listening to resource change event, any time the build job changes (add or remove) the problem markers of the resources under the target project, the handler will sync diagnostics to the client. So it's unnecessary for the BuildWorkspaceHandler to clean up the project level diagnostics before building.

@snjeza
Copy link
Contributor

snjeza commented Nov 21, 2019

test this please

@snjeza
Copy link
Contributor

snjeza commented Nov 21, 2019

Related issue: #1284

@fbricon
Copy link
Contributor

fbricon commented Nov 21, 2019

this was initially added to fix redhat-developer/vscode-java#608. Have you checked what happens with this PR?

@testforstephen
Copy link
Contributor Author

In the original PR redhat-developer/vscode-java#608, the build handler will clean up project level diagnostics before building, but it also has logic to republish all diagnostics to client after building. The entire logic is symmetrical.

But some time later, the publish diagnostics logic is removed from the build handler. See the PR #829. If we still keep cleaning up the project level before build, trigger an incremental build actually won't duplicate report the existing project level errors to WorkspaceDiagnosticsHandler, so the client will probably lose some project level errors.

I think WorkspaceDiagnosticsHandler should be the only entry to sync build result to client.

@testforstephen
Copy link
Contributor Author

A typical reproduce steps:

  1. Open an android gradle project leakcanary in vscode.
  2. After the LS is ready, the PROBLEMS view will show project level error.
  3. Manually Force incremental build, you will see all errors gone from PROBLEMS view.

@snjeza
Copy link
Contributor

snjeza commented Nov 21, 2019

test this please

@fbricon fbricon merged commit 40a007c into eclipse-jdtls:master Nov 25, 2019
@fbricon
Copy link
Contributor

fbricon commented Nov 25, 2019

Thanks @testforstephen!

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

Successfully merging this pull request may close these issues.

3 participants