Ask users to import projects when opening a new folder #1501

jdneo · 2020-06-23T12:53:05Z

Signed-off-by: Sheng Chen sheche@microsoft.com

package.json

fbricon · 2020-06-24T09:55:36Z

src/utils.ts

+	const config = getJavaConfiguration();
+	const isMavenImporterEnabled: boolean = config.get<boolean>("import.maven.enabled");
+	const isGradleImporterEnabled: boolean = config.get<boolean>("import.gradle.enabled");
+	const patterns: string[] = [];
+	if (isMavenImporterEnabled) {
+		patterns.push("**/pom.xml");
+	}
+	if (isGradleImporterEnabled) {
+		patterns.push("**/build.gradle");
+	}


we need to figure out a way to let 3rd party extensions (eg. bazel) register patterns/import prefs from their package.json (something similar to extension jars)

Something like this?

"contributes": { "javaConfigurationFiles": [ { "name": "BUILD", "contentPattern": "java_binary\\s*\\([\\s\\S]*\\)" <--- optional } ], "javaExtensions": [ ... ], ... }

where contentPattern is optional, since for Bazel, the BUILD file can also be used to build other language's project, so we need a regex pattern to test towards the file content.

IMO, you doesn't need test against the file content, because vscode-java won't be automatically activated when detecting BUILD file.

"contributes": { "javaBuildFiles": [ { "fileNamePattern": "BUILD" } ] }

Hard to say... If the third-party extension depends on vscode-java, then it will be activated.

src/utils.ts

src/syntaxLanguageClient.ts

package.json

fbricon · 2020-06-29T08:11:18Z

package.json

@@ -30,8 +30,7 @@
    "onLanguage:java",
    "workspaceContains:pom.xml",
    "workspaceContains:build.gradle",
-    "workspaceContains:.classpath",
-    "onCommand:java.project.import"


why would you remove it? If you start from an empty folder, then add a pom.xml, you'd want to be able to trigger import even though vscode-java hasn't started yet

There is no API to let the extension know which event activates it. When VS Code Java is activated by this command, the user will be asked again to confirm the import again. Is it acceptable?

Maybe a better solution is to let VS Code check the adding files for workspaceContains: microsoft/vscode#28122.

package.json

test/standard-mode-suite/utils.test.ts

README.md

package.json

src/extension.ts

src/syntaxLanguageClient.ts

src/extension.ts

src/syntaxLanguageClient.ts

fbricon · 2020-06-30T09:13:03Z

@jdneo can you please fix the conflict?

Signed-off-by: Sheng Chen <sheche@microsoft.com>

jdneo · 2020-06-30T09:21:06Z

@fbricon Sure, rebased.

src/extension.ts

fbricon · 2020-06-30T10:12:05Z

Everytime I close the vscode window, the syntax server fails to stop on its own, looks like it never receives the shutdown request

Signed-off-by: Sheng Chen <sheche@microsoft.com>

jdneo · 2020-06-30T10:56:29Z

I tried using jps after closing the vs code and not found any language server process. @testforstephen Could you reproduce that?

akaroml

Everytime I close the vscode window, the syntax server fails to stop on its own, looks like it never receives the shutdown request

+1, I observed similar behavior before. Not sure if it's related to this change but I would suggest that we work on a safety net that ensures all Java processes created by vscode-java are closed upon exit. Maybe in a separate PR?

src/extension.ts

testforstephen · 2020-07-01T05:22:02Z

I tried using jps after closing the vs code and not found any language server process. @testforstephen Could you reproduce that?

I observed the lightweight server is still running for a while after i forced to close VS Code window. However, it would automatically exit after about 1 minute.

Below is the log.

!ENTRY org.eclipse.jdt.ls.core 1 0 2020-07-01 13:20:12.657
!MESSAGE Parent process stopped running, forcing server exit

!ENTRY org.eclipse.jdt.ls.core 1 0 2020-07-01 13:20:12.658
!MESSAGE Shutdown received... waking up main thread

!ENTRY org.eclipse.jdt.ls.core 1 0 2020-07-01 13:20:12.665
!MESSAGE class org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin is stopping:

!ENTRY org.eclipse.core.resources 2 10035 2020-07-01 13:20:12.678
!MESSAGE The workspace will exit with unsaved changes in this session.

testforstephen · 2020-07-01T05:29:29Z

The Standard mode looks like the same behavior as "Parent process stopped running, forcing server exit".

Signed-off-by: Sheng Chen <sheche@microsoft.com>

fbricon · 2020-07-01T09:54:32Z

I see the standard server receiving

!ENTRY org.eclipse.jdt.ls.core 1 0 2020-07-01 11:53:35.884
!MESSAGE >> shutdown

!ENTRY org.eclipse.jdt.ls.core 1 0 2020-07-01 11:53:35.923
!MESSAGE >> exit

but never for the syntax server

fbricon · 2020-07-01T10:10:29Z

From the client log, I can see the shutdown command is never sent for the syntax client, but it is for the standard client, which explains my previous comment. But looking at the code, I don't see why this is not working the same for both.
Very easy to reproduce the issue: simply execute the reload window command (Cmd+R when running in dev mode).

fbricon · 2020-07-01T10:13:10Z

The server shutting down on its own after about 30 sec is thanks to https://github.com/eclipse/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/ParentProcessWatcher.java#L49

snjeza · 2020-07-01T18:20:12Z

@fbricon @testforstephen @akaroml @jdneo could you try the following Java LS patch:

diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java
index a6710c75..d2e23a1a 100644
--- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java
+++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java
@@ -161,6 +161,29 @@ public class JDTLanguageServer extends BaseJDTLanguageServer implements Language
 
 	private ProgressReporterManager progressReporterManager;
 
+	private Job shutdownJob = new Job("Shutdown...") {
+
+		@Override
+		protected IStatus run(IProgressMonitor monitor) {
+			try {
+				JavaRuntime.removeVMInstallChangedListener(jvmConfigurator);
+				if (workspaceDiagnosticsHandler != null) {
+					workspaceDiagnosticsHandler.removeResourceChangeListener();
+					workspaceDiagnosticsHandler = null;
+				}
+				if (classpathUpdateHandler != null) {
+					classpathUpdateHandler.removeElementChangeListener();
+					classpathUpdateHandler = null;
+				}
+				ResourcesPlugin.getWorkspace().save(true, monitor);
+			} catch (CoreException e) {
+				logException(e.getMessage(), e);
+			}
+			return Status.OK_STATUS;
+		}
+
+	};
+
 	@Override
 	public LanguageServerWorkingCopyOwner getWorkingCopyOwner() {
 		return workingCopyOwner;
@@ -348,20 +371,7 @@ public class JDTLanguageServer extends BaseJDTLanguageServer implements Language
 	public CompletableFuture<Object> shutdown() {
 		logInfo(">> shutdown");
 		return computeAsync((monitor) -> {
-			try {
-				JavaRuntime.removeVMInstallChangedListener(jvmConfigurator);
-				if (workspaceDiagnosticsHandler != null) {
-					workspaceDiagnosticsHandler.removeResourceChangeListener();
-					workspaceDiagnosticsHandler = null;
-				}
-				if (classpathUpdateHandler != null) {
-					classpathUpdateHandler.removeElementChangeListener();
-					classpathUpdateHandler = null;
-				}
-				ResourcesPlugin.getWorkspace().save(true, monitor);
-			} catch (CoreException e) {
-				logException(e.getMessage(), e);
-			}
+			shutdownJob.schedule();
 			return new Object();
 		});
 	}
@@ -372,6 +382,11 @@ public class JDTLanguageServer extends BaseJDTLanguageServer implements Language
 	@Override
 	public void exit() {
 		logInfo(">> exit");
+		try {
+			shutdownJob.join();
+		} catch (InterruptedException e) {
+			JavaLanguageServerPlugin.logException(e.getMessage(), e);
+		}
 		JavaLanguageServerPlugin.getLanguageServer().exit();
 		Executors.newSingleThreadScheduledExecutor().schedule(() -> {
 			logInfo("Forcing exit after 1 min.");
diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/syntaxserver/SyntaxLanguageServer.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/syntaxserver/SyntaxLanguageServer.java
index 44daf7f1..d3582e89 100644
--- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/syntaxserver/SyntaxLanguageServer.java
+++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/syntaxserver/SyntaxLanguageServer.java
@@ -29,7 +29,9 @@ import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.OperationCanceledException;
+import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.jdt.ls.core.internal.BaseJDTLanguageServer;
 import org.eclipse.jdt.ls.core.internal.JDTUtils;
@@ -92,6 +94,19 @@ public class SyntaxLanguageServer extends BaseJDTLanguageServer implements Langu
 	private ContentProviderManager contentProviderManager;
 	private ProjectsManager projectsManager;
 	private PreferenceManager preferenceManager;
+	private Job shutdownJob = new Job("Shutdown...") {
+
+		@Override
+		protected IStatus run(IProgressMonitor monitor) {
+			try {
+				ResourcesPlugin.getWorkspace().save(true, monitor);
+			} catch (CoreException e) {
+				logException(e.getMessage(), e);
+			}
+			return Status.OK_STATUS;
+		}
+
+	};
 
 	public SyntaxLanguageServer(ContentProviderManager contentProviderManager,
 								ProjectsManager projectsManager,
@@ -120,12 +135,7 @@ public class SyntaxLanguageServer extends BaseJDTLanguageServer implements Langu
 	public CompletableFuture<Object> shutdown() {
 		logInfo(">> shutdown");
 		return computeAsync((monitor) -> {
-			try {
-				ResourcesPlugin.getWorkspace().save(true, monitor);
-			} catch (CoreException e) {
-				logException(e.getMessage(), e);
-			}
-
+			shutdownJob.schedule();
 			return new Object();
 		});
 	}
@@ -133,6 +143,11 @@ public class SyntaxLanguageServer extends BaseJDTLanguageServer implements Langu
 	@Override
 	public void exit() {
 		logInfo(">> exit");
+		try {
+			shutdownJob.join();
+		} catch (InterruptedException e) {
+			JavaLanguageServerPlugin.logException(e.getMessage(), e);
+		}
 		JavaLanguageServerPlugin.getLanguageServer().exit();
 		Executors.newSingleThreadScheduledExecutor().schedule(() -> {
 			System.exit(1);

snjeza · 2020-07-01T18:45:52Z

The issue happens when ResourcesPlugin.getWorkspace().save(true, monitor) takes more than five seconds in which case VS Code doesn't send the exit command.

jdneo · 2020-07-02T02:28:30Z

OK, I can now repro this on Mac. Seems this issue is not reproduceable on Windows.

jdneo · 2020-07-02T03:29:41Z

@snjeza Thank you for the patch! It works while it has a limitation. It ensures the language serve will be shut down if the initialize request is finished.

If the VS Code is closed before the initialize response is sent from the language server, the client will not send the shutdown event to the server -- which is a restriction of LSP. Thus the servers will still be running. (to simply repro this, you can close the VS Code at the moment when the spinning icon shows at the staus-bar).

// BTW, I suggest to discuss this topic in another new issue thread, since it's out of this PR's scope.

fbricon · 2020-07-02T13:22:38Z

so I found out the extension.deactivate() function is never called when the server is started in hybrid mode but the projects have not been imported. So the syntax server client is never stopped, so never sends the shutdown signal.

I just can't figure out why

src/extension.ts

fbricon · 2020-07-02T17:16:46Z

@fbricon @testforstephen @akaroml @jdneo could you try the following Java LS patch:

please open a PR in jdt.ls, that's a different issue

Signed-off-by: Sheng Chen <sheche@microsoft.com>

jdneo · 2020-07-03T05:47:50Z

In case the user may not select an option when asking import. I did some more changes to make sure the vscode java will resolve the API when the asking is needed.

src/extension.ts

Signed-off-by: Sheng Chen <sheche@microsoft.com>

src/syntaxLanguageClient.ts

Signed-off-by: Sheng Chen <sheche@microsoft.com>

fbricon · 2020-07-03T07:42:56Z

src/extension.ts

+
+async function promptUserForStandardServer(config: WorkspaceConfiguration): Promise<boolean> {
+	const choice: string = await window.showInformationMessage("The workspace contains Java projects, would you like to import them?", "Yes", "Always", "Later");
+	switch (choice) {


should we add Never?

Hmm... Personal option is not including Never here. Since our final goal is to let users to use all the features of VS Code Java.

fbricon · 2020-07-03T15:46:34Z

Thanks @jdneo!

snjeza · 2020-07-03T16:26:58Z

please open a PR in jdt.ls, that's a different issue

@fbricon See eclipse-jdtls/eclipse.jdt.ls#1495

jdneo mentioned this pull request Jun 23, 2020

Prompt the user for importing during initialization #1418

Closed

fbricon reviewed Jun 24, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

fbricon reviewed Jun 24, 2020

View reviewed changes

testforstephen requested changes Jun 28, 2020

View reviewed changes

src/utils.ts Outdated Show resolved Hide resolved

src/utils.ts Outdated Show resolved Hide resolved

src/syntaxLanguageClient.ts Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

fbricon reviewed Jun 29, 2020

View reviewed changes

fbricon requested changes Jun 29, 2020

View reviewed changes

testforstephen requested changes Jun 29, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

src/extension.ts Outdated Show resolved Hide resolved

src/syntaxLanguageClient.ts Outdated Show resolved Hide resolved

src/extension.ts Outdated Show resolved Hide resolved

jdneo force-pushed the cs/prompt-import2 branch 2 times, most recently from 0c5653a to a3d5266 Compare June 30, 2020 06:42

testforstephen reviewed Jun 30, 2020

View reviewed changes

src/syntaxLanguageClient.ts Show resolved Hide resolved

jdneo added 5 commits June 30, 2020 17:14

Ask users to import for a fresh workspace

fe30759

Signed-off-by: Sheng Chen <sheche@microsoft.com>

Add the document of the new settings in README

cde8ada

Signed-off-by: Sheng Chen <sheche@microsoft.com>

Address the comments

82c9e44

Signed-off-by: Sheng Chen <sheche@microsoft.com>

Correct some typos

28a9829

Signed-off-by: Sheng Chen <sheche@microsoft.com>

Fix the wording issues

7a79dac

Signed-off-by: Sheng Chen <sheche@microsoft.com>

jdneo force-pushed the cs/prompt-import2 branch from a3d5266 to 7a79dac Compare June 30, 2020 09:15

testforstephen approved these changes Jun 30, 2020

View reviewed changes

fbricon reviewed Jun 30, 2020

View reviewed changes

src/extension.ts Outdated Show resolved Hide resolved

Update the wording

4a29dcc

Signed-off-by: Sheng Chen <sheche@microsoft.com>

akaroml reviewed Jul 1, 2020

View reviewed changes

src/extension.ts Outdated Show resolved Hide resolved

Change the order of the options

26f2727

Signed-off-by: Sheng Chen <sheche@microsoft.com>

fbricon reviewed Jul 2, 2020

View reviewed changes

src/extension.ts Outdated Show resolved Hide resolved

snjeza mentioned this pull request Jul 2, 2020

Improve Java LS shutdown eclipse-jdtls/eclipse.jdt.ls#1495

Merged

Resolve the API in LightWeight mode when asking user to import

e446e0b

Signed-off-by: Sheng Chen <sheche@microsoft.com>

fbricon requested changes Jul 3, 2020

View reviewed changes

src/extension.ts Outdated Show resolved Hide resolved

resolve the API inside activate()

bb834b0

Signed-off-by: Sheng Chen <sheche@microsoft.com>

fbricon reviewed Jul 3, 2020

View reviewed changes

src/syntaxLanguageClient.ts Outdated Show resolved Hide resolved

Add return type

c252728

Signed-off-by: Sheng Chen <sheche@microsoft.com>

fbricon reviewed Jul 3, 2020

View reviewed changes

fbricon changed the title ~~Ask users to import for a fresh workspace~~ Ask users to import projects when opening a new folder Jul 3, 2020

fbricon merged commit 0c4055b into redhat-developer:master Jul 3, 2020

fbricon added this to the End June 2020 milestone Jul 3, 2020

fbricon added the enhancement label Jul 3, 2020

jdneo deleted the cs/prompt-import2 branch July 4, 2020 02:33

jdneo mentioned this pull request Sep 24, 2020

Prompt users to input during initialization … eclipse-jdtls/eclipse.jdt.ls#1427

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ask users to import projects when opening a new folder #1501

Ask users to import projects when opening a new folder #1501

jdneo commented Jun 23, 2020

fbricon Jun 24, 2020 •

edited

jdneo Jun 24, 2020

testforstephen Jun 28, 2020 •

edited

jdneo Jun 28, 2020

fbricon Jun 29, 2020

jdneo Jun 29, 2020

fbricon commented Jun 30, 2020

jdneo commented Jun 30, 2020

fbricon commented Jun 30, 2020

jdneo commented Jun 30, 2020

akaroml left a comment

testforstephen commented Jul 1, 2020

testforstephen commented Jul 1, 2020

fbricon commented Jul 1, 2020

fbricon commented Jul 1, 2020

fbricon commented Jul 1, 2020

snjeza commented Jul 1, 2020

snjeza commented Jul 1, 2020

jdneo commented Jul 2, 2020

jdneo commented Jul 2, 2020

fbricon commented Jul 2, 2020

fbricon commented Jul 2, 2020

jdneo commented Jul 3, 2020

fbricon Jul 3, 2020

jdneo Jul 3, 2020

fbricon commented Jul 3, 2020

snjeza commented Jul 3, 2020

Ask users to import projects when opening a new folder #1501

Ask users to import projects when opening a new folder #1501

Conversation

jdneo commented Jun 23, 2020

fbricon Jun 24, 2020 • edited

Choose a reason for hiding this comment

jdneo Jun 24, 2020

Choose a reason for hiding this comment

testforstephen Jun 28, 2020 • edited

Choose a reason for hiding this comment

jdneo Jun 28, 2020

Choose a reason for hiding this comment

fbricon Jun 29, 2020

Choose a reason for hiding this comment

jdneo Jun 29, 2020

Choose a reason for hiding this comment

fbricon commented Jun 30, 2020

jdneo commented Jun 30, 2020

fbricon commented Jun 30, 2020

jdneo commented Jun 30, 2020

akaroml left a comment

Choose a reason for hiding this comment

testforstephen commented Jul 1, 2020

testforstephen commented Jul 1, 2020

fbricon commented Jul 1, 2020

fbricon commented Jul 1, 2020

fbricon commented Jul 1, 2020

snjeza commented Jul 1, 2020

snjeza commented Jul 1, 2020

jdneo commented Jul 2, 2020

jdneo commented Jul 2, 2020

fbricon commented Jul 2, 2020

fbricon commented Jul 2, 2020

jdneo commented Jul 3, 2020

fbricon Jul 3, 2020

Choose a reason for hiding this comment

jdneo Jul 3, 2020

Choose a reason for hiding this comment

fbricon commented Jul 3, 2020

snjeza commented Jul 3, 2020

fbricon Jun 24, 2020 •

edited

testforstephen Jun 28, 2020 •

edited