Update java support to latest jdt.ls #1583

gorkem · 2018-03-27T01:45:22Z

Updates the server to the latest snapshot which includes
a fix that is required for Theia. Also changes the way
language server is started to adjust to the changes
on environment variables.

fixes #1068 also improves #1332

akosyakov · 2018-03-27T06:22:27Z

packages/java/src/browser/java-client-contribution.ts

@@ -8,44 +8,44 @@
 import { injectable, inject } from "inversify";
 import { CommandService } from "@theia/core/lib/common";
 import {
-    Window, ILanguageClient, BaseLanguageClientContribution, Workspace, Languages, LanguageClientFactory
+  Window, ILanguageClient, BaseLanguageClientContribution, Workspace, Languages, LanguageClientFactory


Could you check your formatting setting please? It is 4 spaces for ts files.
For VS code we have it checked in: https://github.com/theia-ide/theia/blob/e41a77e57de7f5948e3c61cfe5f5f724b6e2fac1/.vscode/settings.json#L41

Naturally I use Theia to develop Theia. I think it is odd that we are depending on VSCode on these.

very good :) but Theia defaults is 4 spaces: https://github.com/theia-ide/theia/blob/f4de4f572bdebd7dadae5e7a3323e39fb346d566/packages/editor/src/browser/editor-preferences.ts#L24

We can check in .theia/setting.json as well.

kittaakos · 2018-03-27T09:50:41Z

packages/java/src/node/java-contribution.ts

+    Promise.all(
+      [this.startSocketServer()]
+    ).then(servers => {
+      const [server] = servers;


Minor: perhaps, you could get rid of the array of servers and the Promise.all as well if we have one, right? And write

this.startSocketServer().then(server => { // blabla });

instead.

marcdumais-work · 2018-03-27T14:12:40Z

@gorkem I gave this PR a quick try on Linux. I can confirm that now "Format Document" seems to work, and it did not on master.

However the diagnostics, at least on Linux, seem broken. Whatever I type, it seems that no new error markers are created ("old" ones I think were saved in the browser and restored but can't be cleared by fixing the issue). That works better on master.

Completion seems not to work as well also, with this PR.

marcdumais-work · 2018-03-27T14:19:25Z

However the diagnostics, at least on Linux, seem broken.

Actually, not completely broken - the diagnostics seem to kick-in once in a while.

gorkem · 2018-03-27T15:22:47Z

@marcdumais-work correct. That is because for some reason that I have not discovered yet jdt.ls is not receiveing didOpen/didChange/didClose events. Those events are needed to update the java AST model. At the moment the diagnostics are happening because of a side-effect of other events.

kittaakos · 2018-03-27T16:00:51Z

I have restarted the Windows CI.

gorkem · 2018-03-28T18:37:44Z

Fixed comments and rebased to master should be ready to go.

marcdumais-work · 2018-03-29T11:08:30Z

@gorkem what about completion? It still seems not to work well, with the latest version of this PR.

However I re-tried on master and it's now not working at all - maybe I was lucky it worked yesterday. So overall this PR is not making things much worse, so no objection from me to merge. I hope you can continue to improve Java LS support.

marcdumais-work · 2018-03-29T11:09:13Z

BTW the Travis failures seemed unrelated - I have restarted that build.

gorkem · 2018-03-29T17:33:22Z

@marcdumais-work not working at all is not normal, I will check again. The other oddities are expected because jdt.ls is still not receiving any of the TextDocumentSync events so it is likely that the Java model is out of sync with the editing.

marcdumais-work · 2018-03-29T18:11:17Z

@gorkem the completions "not working at all" was when I tried on master today, not with this PR. It was working yesterday for me on master, so maybe something is flaky.

gorkem · 2018-03-30T16:48:23Z

finally found the culprit. jdt.ls is hitting the bug fixed with TypeFox/vscode-languageserver-node@e0c1f19. @svenefftinge When can we update the library?

svenefftinge · 2018-04-01T09:56:54Z

We want to bump to the latest monaco, that would include rebasing this library, as well. But it is a bit more work. But we can for sore make a minor patch release of 'vscode-base-languageclient' that just includes that fix.

AlexTugarev · 2018-04-03T08:22:18Z

@svenefftinge, may I do the patch release?

AlexTugarev · 2018-04-03T13:52:42Z

packages/java/src/node/java-contribution.ts

@@ -48,7 +48,7 @@ export class JavaContribution extends BaseLanguageServerContribution {

        if (DEBUG_MODE) {
            args.push(
-                '-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=1044',
+                '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=1044',


@gorkem, do we really want to suspend the Jvm? I guess it's useful to debug the LS handshake. Just asking.

Good catch. It should be fixed now.

AlexTugarev · 2018-04-03T13:54:24Z

@gorkem this looks good to me. We need those changes!

On the vscode-base-languageclient, I've published new version containing the bugfix. Now updating the chain in order to consume it in Theia. #1539

AlexTugarev

LGTM!

AlexTugarev · 2018-04-05T16:40:31Z

@gorkem, the CI build is passed, do you want to merge?

gorkem · 2018-04-05T16:57:49Z

OK, I will rebase and merge

AlexTugarev · 2018-04-06T10:00:45Z

@gorkem, not whether how this is related, but trying out latest changes shows that the java editor is broken. Looking at the errors I see at begins with trying to register java.edit.organizeImports command, and the error message is that's already registered. Then I see lot's of follow-up errors. Trying to complete System.o|ut shows sometimes errors like INVALID range, must be single line and on the same line.

Suggestions seems to be off, too:

Then I see protocol error messages like:

"Apr 6, 2018 11:57:51 AM Failed to create linked resource from file:///Users/tugarev/Projects/web2/Main123.java to jdt.ls-java-project
Resource '/jdt.ls-java-project/src/Main123.java' already exists.
org.eclipse.core.internal.resources.ResourceException(/jdt.ls-java-project/src/Main123.java)[367]: java.lang.Exception: Resource '/jdt.ls-java-project/src/Main123.java' already exists.
	at org.eclipse.core.internal.resources.ResourceException.provideStackTrace(ResourceException.java:39)
	at org.eclipse.core.internal.resources.ResourceException.<init>(ResourceException.java:35)
	at org.eclipse.core.internal.resources.Workspace.createResource(Workspace.java:1362)
	at org.eclipse.core.internal.resources.Workspace.createResource(Workspace.java:1297)
	at org.eclipse.core.internal.resources.Resource.createLink(Resource.java:636)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.getFakeCompilationUnit(JDTUtils.java:207)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.resolveCompilationUnit(JDTUtils.java:140)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.resolveCompilationUnit(JDTUtils.java:113)
	at org.eclipse.jdt.ls.core.internal.handlers.CodeActionHandler.getCodeActionCommands(CodeActionHandler.java:59)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.lambda$12(JDTLanguageServer.java:515)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.lambda$21(JDTLanguageServer.java:703)
	at java.util.concurrent.CompletableFuture.uniApply(CompletableFuture.java:602)
	at java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:577)
	at java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:443)
	at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
	at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
	at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
	at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
"

Do you have any issues?

gorkem · 2018-04-06T15:52:20Z

@AlexTugarev I think you are hitting an issue that jdt.ls faces time to time. This usually happens when we change eclipse versions or a workspace (the eclipse one) is somewhat corrupted. Deleting the workspace should fix it

Updates the server to the latest snapshot which includes a fix that is required for Theia. Also changes the way language server is started to adjust to the changes on environment variables. fixes eclipse-theia#1068 Signed-off-by: Gorkem Ercan <[email protected]>

gorkem · 2018-04-09T17:11:29Z

Rebased and updated jdt.ls version

AlexTugarev · 2018-04-09T17:25:57Z

Thanks for the hint. I’ll try it.

AlexTugarev · 2018-04-10T07:55:18Z

@gorkem, I tried with fresh project and made sure there is no temp workspace data picked up.

The Java editor is still not useable, and it most likely because of the missing TextDocumentSync. This gif shows how the LS is some changes behind the editor:

But I also still see errors like "java.edit.organizeImports" (with a lot of follow ups). I just verified this by patching this._capabilites.textDocumentSync to 2 in /vscode-base-languageclient/lib/base.js. Also code lenses seems to disappear and reappear while typing, which is very disturbing.

Could you check the "java.edit.organizeImports" issue?

gorkem · 2018-04-10T12:21:52Z

@AlexTugarev Where do you see the java.edit.organizeImports issue?

Also code lenses seems to disappear and reappear while typing, which is very disturbing.
I think we need a separate issue for it. It is probably a general issue.

gorkem · 2018-04-10T18:06:29Z

java.edit.organizeImports looks like it is registered multiple times because jdt.ls dynamically registers it to enable 'workspace/executeCommand' calls. However it is also registered with a 'CommandContribution' at startup to enable Menu and keybinding registration. I do not think there is anything jdt.ls can do for it. LSP implementation should probably be changed to avoid re-registration.

gorkem · 2018-04-11T00:27:05Z

created #1684 for the multiple command registration issue.

AlexTugarev · 2018-04-16T16:02:27Z

@gorkem, I've created this issue for JDT LS eclipse-jdtls/eclipse.jdt.ls#629. I think this is responsible for the hiccups, which appears when typing.

AlexTugarev

LGTM!

I think we've identified the necessary follow up issues to improve the UX of the Java extension.

gorkem force-pushed the java_update branch from 0f2b96d to 4951131 Compare March 27, 2018 01:46

akosyakov reviewed Mar 27, 2018

View reviewed changes

kittaakos reviewed Mar 27, 2018

View reviewed changes

gorkem force-pushed the java_update branch 3 times, most recently from 41fb776 to 190e381 Compare March 28, 2018 18:36

AlexTugarev reviewed Apr 3, 2018

View reviewed changes

gorkem force-pushed the java_update branch from 190e381 to cbedf7f Compare April 3, 2018 15:34

AlexTugarev previously approved these changes Apr 4, 2018

View reviewed changes

gorkem dismissed AlexTugarev’s stale review via 0e7edb2 April 5, 2018 17:45

gorkem force-pushed the java_update branch from cbedf7f to 0e7edb2 Compare April 5, 2018 17:45

gorkem force-pushed the java_update branch from 0e7edb2 to 187c6ce Compare April 9, 2018 16:27

AlexTugarev mentioned this pull request Apr 10, 2018

update monaco editor to 0.12.x #1681

Merged

3 tasks

AlexTugarev approved these changes Apr 16, 2018

View reviewed changes

gorkem merged commit e349656 into eclipse-theia:master Apr 16, 2018

This was referenced Apr 19, 2018

Remove jdt.ls from repository #1732

Merged

Consider reusing the same jdt.ls workspace on restart (for a given opened folder) #1745

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update java support to latest jdt.ls #1583

Update java support to latest jdt.ls #1583

gorkem commented Mar 27, 2018

akosyakov Mar 27, 2018

gorkem Mar 27, 2018

akosyakov Mar 27, 2018 •

edited

Loading

kittaakos Mar 27, 2018

marcdumais-work commented Mar 27, 2018

marcdumais-work commented Mar 27, 2018

gorkem commented Mar 27, 2018

kittaakos commented Mar 27, 2018

gorkem commented Mar 28, 2018

marcdumais-work commented Mar 29, 2018

marcdumais-work commented Mar 29, 2018

gorkem commented Mar 29, 2018 •

edited

Loading

marcdumais-work commented Mar 29, 2018

gorkem commented Mar 30, 2018

svenefftinge commented Apr 1, 2018 •

edited

Loading

AlexTugarev commented Apr 3, 2018

AlexTugarev Apr 3, 2018

gorkem Apr 3, 2018

AlexTugarev commented Apr 3, 2018

AlexTugarev left a comment

AlexTugarev commented Apr 5, 2018

gorkem commented Apr 5, 2018

AlexTugarev commented Apr 6, 2018

gorkem commented Apr 6, 2018

gorkem commented Apr 9, 2018

AlexTugarev commented Apr 9, 2018

AlexTugarev commented Apr 10, 2018

gorkem commented Apr 10, 2018

gorkem commented Apr 10, 2018 •

edited

Loading

gorkem commented Apr 11, 2018

AlexTugarev commented Apr 16, 2018

AlexTugarev left a comment

Update java support to latest jdt.ls #1583

Update java support to latest jdt.ls #1583

Conversation

gorkem commented Mar 27, 2018

akosyakov Mar 27, 2018

Choose a reason for hiding this comment

gorkem Mar 27, 2018

Choose a reason for hiding this comment

akosyakov Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

kittaakos Mar 27, 2018

Choose a reason for hiding this comment

marcdumais-work commented Mar 27, 2018

marcdumais-work commented Mar 27, 2018

gorkem commented Mar 27, 2018

kittaakos commented Mar 27, 2018

gorkem commented Mar 28, 2018

marcdumais-work commented Mar 29, 2018

marcdumais-work commented Mar 29, 2018

gorkem commented Mar 29, 2018 • edited Loading

marcdumais-work commented Mar 29, 2018

gorkem commented Mar 30, 2018

svenefftinge commented Apr 1, 2018 • edited Loading

AlexTugarev commented Apr 3, 2018

AlexTugarev Apr 3, 2018

Choose a reason for hiding this comment

gorkem Apr 3, 2018

Choose a reason for hiding this comment

AlexTugarev commented Apr 3, 2018

AlexTugarev left a comment

Choose a reason for hiding this comment

AlexTugarev commented Apr 5, 2018

gorkem commented Apr 5, 2018

AlexTugarev commented Apr 6, 2018

gorkem commented Apr 6, 2018

gorkem commented Apr 9, 2018

AlexTugarev commented Apr 9, 2018

AlexTugarev commented Apr 10, 2018

gorkem commented Apr 10, 2018

gorkem commented Apr 10, 2018 • edited Loading

gorkem commented Apr 11, 2018

AlexTugarev commented Apr 16, 2018

AlexTugarev left a comment

Choose a reason for hiding this comment

akosyakov Mar 27, 2018 •

edited

Loading

gorkem commented Mar 29, 2018 •

edited

Loading

svenefftinge commented Apr 1, 2018 •

edited

Loading

gorkem commented Apr 10, 2018 •

edited

Loading