Don't throw an exception if none LS was found#11094
Conversation
svor
added
kind/bug
tsmaeder
left a comment
tsmaeder
left a comment
There was a problem hiding this comment.
Generally, I think we should do most of the work in the back end.
| e -> { | ||
| subscribe(); | ||
| hasJDTLSInstallerAdded = checkJDTLSInstaller(); | ||
| initializeJDTLS(); |
There was a problem hiding this comment.
If we want to start jdt.ls as soon as the workspace starts, we should do so in wsagent. For example, the current approach would try to initialize jdt.ls from ever open IDE. If we handle immediate startup on the back end, we don't have any such problems.
| WorkspaceReadyEvent.getType(), | ||
| e -> { | ||
| subscribe(); | ||
| hasJDTLSInstallerAdded = checkJDTLSInstaller(); |
There was a problem hiding this comment.
If the workspace is restarted, this might not be true anymore. You can restart the workspace without restarting the IDE:
|
|
||
| @Override | ||
| public Promise<Map<String, String>> loadPreferences() { | ||
| if (!service.hasJDTLSInstallerAdded()) { |
There was a problem hiding this comment.
If you follow the call hierarachy from here, this is only called early to populate CurrentUser#getPreferences(). But that method is never called at all. Maybe we can remove that method?
| if (!service.hasJDTLSInstallerAdded()) { | ||
| return promiseProvider.resolve(newHashMap()); | ||
| } | ||
|
|
There was a problem hiding this comment.
Why are we not ensuring jdt.ls is enabled in the back end like we do for JavaLanguageServerExtensionService#executeCommand()? That would have the same effect, basically and be a one-liner.
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
svor
changed the title
|
@tsmaeder I've updated the PR. Please take another look at it. |
| return doGetOne(Commands.GET_JAVA_CORE_OPTIONS_СOMMAND, new ArrayList<>(filter), type); | ||
| JavaCoreOptions result = | ||
| doGetOne(Commands.GET_JAVA_CORE_OPTIONS_СOMMAND, new ArrayList<>(filter), type); | ||
| return result == null ? new JavaCoreOptions() : result; |
There was a problem hiding this comment.
Why do we need the null check? Shoudn't this return a result or throw an exception?
There was a problem hiding this comment.
It's better to return empty map of java compiler options than null, if jdt.ls wasn't initialized
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
benoitf
removed
the
status/code-review
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>
4 participants
Signed-off-by: Valeriy Svydenko vsvydenk@redhat.com
What does this PR do?
What issues does this PR fix or reference?
#11091
Release Notes
Docs PR