Add organize imports command #341

yaohaizh · 2017-10-24T07:23:07Z

Fixes #253

Signed-off-by: Yaohai Zheng yaozheng@microsoft.com

ankon · 2017-10-24T08:05:54Z

src/commands.ts

@@ -67,5 +67,15 @@ export namespace Commands {
 	 /**
 	 * Open Java Language Server Log file
 	 */
-	export const OPEN_SERVER_LOG = 'java.open.serverLog';
+    export const OPEN_SERVER_LOG = 'java.open.serverLog';


Something happened with the indentation here and below.

basically tabs were replaced with spaces. I think we need to set a project preference for indentation. But until then we should try to keep indentation coherent all over the place.
Ultimately we can do a major sweep to format all files at once, but that's the subject of another commit.

I would like to keep the indentation consistent, and will accept either tab or whitespace.

As I went through the vscode-java codebase, there are mixed with tab and whitespaces. We need make them consistent anyway.

gorkem · 2017-10-24T11:22:02Z

Can you also add the new command to tests at https://github.com/redhat-developer/vscode-java/blob/master/test/extension.test.ts#L25

fbricon · 2017-10-24T23:05:44Z

I can't bind a keyboard shortcut to the organize import command from the VSCode UI.

It works when editing the keybindings.json manually.

However, when calling the shortcut on a file with imports to organize, an error occurs on the server:


[Error - 19:07:04] 24-Oct-2017 7:02:16 PM Failed to resolve [object Object]
Illegal character in path at index 0: [object Object]
java.net.URISyntaxException: Illegal character in path at index 0: [object Object]
	at java.net.URI$Parser.fail(URI.java:2848)
	at java.net.URI$Parser.checkChars(URI.java:3021)
	at java.net.URI$Parser.parseHierarchical(URI.java:3105)
	at java.net.URI$Parser.parse(URI.java:3063)
	at java.net.URI.<init>(URI.java:588)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.toURI(JDTUtils.java:591)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.resolveCompilationUnit(JDTUtils.java:107)
	at org.eclipse.jdt.ls.core.internal.JDTDelegateCommandHandler.organizeImports(JDTDelegateCommandHandler.java:50)
	at org.eclipse.jdt.ls.core.internal.JDTDelegateCommandHandler.executeCommand(JDTDelegateCommandHandler.java:39)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceExecuteCommandHandler$1.run(WorkspaceExecuteCommandHandler.java:133)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceExecuteCommandHandler.executeCommand(WorkspaceExecuteCommandHandler.java:128)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.lambda$2(JDTLanguageServer.java:258)
	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)

fbricon · 2017-10-24T23:48:54Z

package.json

@@ -150,6 +150,11 @@
        "command": "java.open.serverLog",
        "title": "Open Java Language Server log file",
        "category": "Java"
+      },
+      {
+        "command": "vscode.java.edit.organizeImports",


Can we skip the vscode prefix? Having all commands under the java namespace is more consistent

fbricon · 2017-10-24T23:49:14Z

package.json

+    "menus": {
+      "editor/context": [
+        {
+          "command": "vscode.java.edit.organizeImports",


again, skip vscode prefix

This cannot be skip. There are two commands here actually.

One is appeared on the VSCode UI side, which is starting with prefix "vscode"

Another one is just starting with "java...", which used for underlying talking with JDT.LS, which is neutral.

fbricon · 2017-10-24T23:50:34Z

src/commands.ts

+    /**
+	 * VSCode UI for organize imports commands
+	 */
+	export const VSCODE_EDIT_ORGANIZE_IMPORTS = 'vscode.java.edit.organizeImports';


Does having the same command id for client and server cause any issue?

fbricon · 2017-10-24T23:51:03Z

src/extension.ts

@@ -160,6 +160,22 @@ export function activate(context: ExtensionContext) {
 						}
 					});

+					commands.registerCommand(Commands.VSCODE_EDIT_ORGANIZE_IMPORTS, async (uri: Uri) => {


need to check the uri is actually an Uri

fbricon · 2017-10-24T23:54:21Z

src/extension.ts

+					commands.registerCommand(Commands.VSCODE_EDIT_ORGANIZE_IMPORTS, async (uri: Uri) => {
+						const edit = await <any>commands.executeCommand(Commands.EXECUTE_WORKSPACE_COMMAND, Commands.EDIT_ORGANIZE_IMPORTS, uri.toString());
+						const workspaceEdit = new WorkspaceEdit();
+


any way to reuse what we have L156?

fbricon · 2017-10-24T23:56:10Z

package.json

+      "editor/context": [
+        {
+          "command": "vscode.java.edit.organizeImports",
+          "when": "resourceLangId == java",


can we call the command from a folder (not just files) in the explorer?

fbricon · 2017-10-27T21:18:04Z

package.json

+      "explorer/context": [
+        {
+          "command": "java.edit.organizeImports",
+          "when": "resourceLangId == java",


can we enable it for folders too?

fbricon · 2017-10-27T21:21:18Z

Any idea why calling a keyboard shortcut doesn't work? With:

{
    "key": "alt+cmd+o",
    "when": "editorTextFocus && !editorReadnly",
    "command": "java.edit.organizeImports"
}

the command receives an empty Object, instead of a URI, so nothing happens

yaohaizh · 2017-10-31T02:39:34Z

It seems the keybinding doesn't provide any parameters as the context menu do. We need change the behavior of these commands.

Signed-off-by: Yaohai Zheng <yaozheng@microsoft.com>

yaohaizh · 2017-10-31T03:18:28Z

For adding commands of "Organize Imports" in folder/workspace in VSCode, there is an issue here. We cannot determine current project(Could we? ). If so, the "Organize Imports" will be appeared in all VSCode instance, which is not desired behavior.

fbricon · 2017-10-31T03:39:16Z

Ok, we can figure out how to get folders working later on the client side. Though I still think we can have it supported on the server side

aeschli · 2017-10-31T08:37:09Z

@yaohaizh If you want to limit the command to only show up for certain files, this is how it works: https://github.com/Microsoft/vscode/blob/cabf4b2997fddf7eabff83d4d22ffa72381545b1/extensions/markdown/package.json#L191

fbricon · 2017-10-31T15:13:20Z

@aeschli that's what @yaohaizh did. The question is how to enable it for folders of java projects?

fbricon · 2017-10-31T23:37:49Z

Merged as 9779734 (with keybinding to Shift+Alt+o)

gorkem added the in progress label Oct 24, 2017

yaohaizh mentioned this pull request Oct 24, 2017

Add organize imports eclipse-jdtls/eclipse.jdt.ls#406

Merged

ankon reviewed Oct 24, 2017

View reviewed changes

fbricon reviewed Oct 24, 2017

View reviewed changes

yaohaizh force-pushed the yaohai_dev branch from 082e112 to e329891 Compare October 25, 2017 05:52

fbricon reviewed Oct 27, 2017

View reviewed changes

Add organize imports command

083d440

Signed-off-by: Yaohai Zheng <yaozheng@microsoft.com>

yaohaizh force-pushed the yaohai_dev branch from e329891 to 083d440 Compare October 31, 2017 03:14

yaohaizh changed the title ~~Add organize imports command[WIP]~~ Add organize imports command Oct 31, 2017

fbricon closed this Oct 31, 2017

yaohaizh deleted the yaohai_dev branch November 1, 2017 09:24

fbricon added this to the End October 2017 milestone Nov 1, 2017

fbricon added enhancement and removed in progress labels Nov 1, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add organize imports command #341

Add organize imports command #341

yaohaizh commented Oct 24, 2017 •

edited by fbricon

ankon Oct 24, 2017

fbricon Oct 24, 2017

yaohaizh Oct 25, 2017

gorkem commented Oct 24, 2017

fbricon commented Oct 24, 2017 •

edited

fbricon Oct 24, 2017 •

edited

fbricon Oct 24, 2017

yaohaizh Oct 25, 2017

fbricon Oct 24, 2017

fbricon Oct 24, 2017

fbricon Oct 24, 2017

fbricon Oct 24, 2017 •

edited

fbricon Oct 27, 2017

fbricon commented Oct 27, 2017

yaohaizh commented Oct 31, 2017

yaohaizh commented Oct 31, 2017

fbricon commented Oct 31, 2017

aeschli commented Oct 31, 2017 •

edited

fbricon commented Oct 31, 2017

fbricon commented Oct 31, 2017

Add organize imports command #341

Add organize imports command #341

Conversation

yaohaizh commented Oct 24, 2017 • edited by fbricon

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gorkem commented Oct 24, 2017

fbricon commented Oct 24, 2017 • edited

fbricon Oct 24, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbricon Oct 24, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbricon commented Oct 27, 2017

yaohaizh commented Oct 31, 2017

yaohaizh commented Oct 31, 2017

fbricon commented Oct 31, 2017

aeschli commented Oct 31, 2017 • edited

fbricon commented Oct 31, 2017

fbricon commented Oct 31, 2017

yaohaizh commented Oct 24, 2017 •

edited by fbricon

fbricon commented Oct 24, 2017 •

edited

fbricon Oct 24, 2017 •

edited

fbricon Oct 24, 2017 •

edited

aeschli commented Oct 31, 2017 •

edited