Alternative ProjectProblems presenter for LSP server. #3568
Conversation
sdedic
added
enhancement
LSP
.../java.lsp.server/src/org/netbeans/modules/java/lsp/server/project/BrokenReferencesModel.java
Show resolved
Hide resolved
.../java.lsp.server/src/org/netbeans/modules/java/lsp/server/project/ProjectAlertPresenter.java
Show resolved
Hide resolved
.../java.lsp.server/src/org/netbeans/modules/java/lsp/server/project/ProjectAlertPresenter.java
Outdated
Show resolved
Hide resolved
sdedic
force-pushed
the
projects/broken-references
branch
from
e02342c
to
325c279
Compare
sdedic
force-pushed
the
projects/broken-references
branch
from
d3fe07f
to
8789e44
Compare
|
Consolidated fixes ... actually everything into a single commit. |
|
I'd like to hold this PR for a moment - during testing, I've encountered NETBEANS-5846 this time even in vscode; together with this PR, NETBEANS-5846 looks rather ugly (several dialogs displayed on project open). I'd prefer to fix NETBEANS-5846 first the way @lkishalmi suggested there. Tracked as #3668 |
sdedic
added
do not merge
|
Prerequisite PR #3668 merged, so this is ready. |
|
Just random failures from ServerTest / ProjectViewTest. Merging. |
ide/projectuiapi.base/src/org/netbeans/spi/project/ui/ProjectProblemsProvider.java
Outdated
Show resolved
Hide resolved
| * displayed | ||
| * @author sdedic | ||
| */ | ||
| // BAD BAD BAD - DO NOT REGISTER THIS IN LSP SERVER, put it in the integration module !!! |
There was a problem hiding this comment.
Would making this class abstract with protected constructor help to remove the BAD BAD message?
There was a problem hiding this comment.
But how can the integration module access this class, when it is not in public package?
There was a problem hiding this comment.
Would making this class abstract with protected constructor help to remove the BAD BAD message?
It would not :) it's there just for case that someone won't get the great idea to add @ServiceProvider since the lsp.server module is a part of NetBeans IDE distro and this override would screw the IDE up.
The integration module uses an explicit META-INF/Services registration that references this class by name. For that the default constructor has to be public. If too ugly, the alternative would be to expose a factory method for the implementation as an API, but I really do not want to expose the implementation class in the APIs.
There was a problem hiding this comment.
There is a common pattern used for this kind of registrations and there are multiple AbstractLspXYZ classes. What is so special on this class that requires a special treatment?
Unless you have some intrinsic answer to the above question, then I suggest to create AbstractLspProjectProblemsImplementation, make it abstract, have protected constructor like other implementations do. However, it is just a suggestion, do as you like.
There was a problem hiding this comment.
Dear colleague(s). Your peristence in commenting my laziness finally worked... So this should be fixed in 033b943
sdedic
force-pushed
the
projects/broken-references
branch
from
39596e2
to
033b943
Compare
| * @since 1.19 | ||
| * @author sdedic | ||
| */ | ||
| public abstract class AbstractLspBrokenReferences implements ProjectProblemsImplementation { |
| private final BrokenReferencesImpl delegate; | ||
|
|
||
| protected AbstractLspBrokenReferences() { | ||
| delegate = new BrokenReferencesImpl(); |
There was a problem hiding this comment.
Possibly check for allowed subclasses in the constructor...
sdedic
force-pushed
the
projects/broken-references
branch
from
033b943
to
540fb3e
Compare
Project problems are presented in a rich UI dialog in NB, but that's not suitable for LSP protocol that only allows for limited dialogs or quickpicks. Instead of a rich-interaction view, this PR replaces (just for the LSP !!) the process by a series of dialogs.
There's a timeout implemented for the user's response to questions or messages; the process will continue after the timeout. This is because the LSP client user may completely ignore (even not cancel) the messages, but I'd still want the identified errors to be displayed.
A
showAlertAPI call will immediately continue the problem resolution process, as if the timeout had elapsed; for example on project reload or reopen. But ony problems not yet presented to the user are processed.A
showCustomizerAPI call will restart the whole process with all the currently known problems (incl. the already shown ones, if they still exist).This alternative implementation is present in
java/java.lsp.servermodule, but the implementation is only registered innbcode/integrationmodule which is part ofnbcodedistribution (published as Apache NetBeans Language Server extension for vscode).