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

Maven error/warnings refactored to use Parsing API. #3793

Merged
merged 1 commit into from
Mar 21, 2022

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Mar 16, 2022

Originally I was trying to bridge project problems to LSP. During the course I have realized that Maven's errors, underlined in the editor, are provided by UpToDateStatusProvider implementation - this one is in charge or reporting start/stop computations in annotation's gutter.
The Provider however monitored on-disk file changes, so potential errors were only reported after file save, not (as usual) when a change to the opened document has been made by the user. This approach bypasses alll the machinery of Parsing API which is more suitable for tracking document changes and/or caret/selection in the live document. In addition, this approach wouldn't work in the LSP client scenario at all.

The StatusProvider implementation was broken anyway as for example StatusProvider kept the status UP_TO_DATE_PROCESSING if there was a non-empty selection ... The UpToDateStatusProvider + hints is generally broken, see NETBEANS-6477. I will not fix that in this PR.

The PR adapts Maven implementation to use Parsing API - this allows to react immediately, and since I provided a custom ModelSource (see M2S class) to maven processing, even the live editor content can be analyzed by the Maven library.

Tests for this module were broken - test data file was missing; I've created some + added necessary env setup so the test passes now.

I've also discovered that Tasklist Maven integration is broken for some time - see NETBEANS-6476, will not fix in this PR either.

@sdedic sdedic self-assigned this Mar 16, 2022
@sdedic sdedic added kind:bug Bug report or fix Maven [ci] enable "build tools" tests labels Mar 16, 2022
@sdedic sdedic added this to the NB14 milestone Mar 16, 2022
Copy link
Member

@ppisl ppisl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not seen an issue.

@sdedic
Copy link
Member Author

sdedic commented Mar 18, 2022

Sorry :( I've accidentally committed here changes that belong to other PR; reverted back to the reviewed state.

Copy link

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice architecture improvement, I think.

Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine. Thanks.

@sdedic sdedic merged commit e26276d into apache:master Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug Bug report or fix Maven [ci] enable "build tools" tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants