Modified to use WorkspaceEdit on OrganizeImports. #1059

mikoto2000 · 2019-06-02T22:37:42Z

This PR is part of #376.

Modified to use WorkspaceEdit on OrganizeImports if client supported workspace/applyEdit.

Needed those client capabilities and initialization parameters for use WorkspaceEdit.

kind: quickfix
- workspace/applyEdit is true
- textDocument/codeAction/codeActionLiteralSupport/codeActionKind/valueSet contains quickfix
kind: source.organizeImports
- workspace/applyEdit is true
- initializationOptions/extendedClientCapabilities/advancedOrganizeImportsSupport is true

eclipse-ls-bot · 2019-06-02T22:37:45Z

Can one of the admins verify this patch?

fbricon · 2019-06-02T22:53:32Z

Test this please

fbricon · 2019-06-02T22:55:34Z

@mikoto2000 can you please sign the Eclipse ECA and sign-off your commit?

mikoto2000 · 2019-06-03T23:31:55Z

Okay. I will sign ECA and add Signed-Off-By.

mikoto2000 · 2019-06-04T12:32:22Z

I added Signed-Off-By.

yaohaizh · 2019-06-09T15:03:04Z

This might break the organize imports if the organize coming with commands such as resolve ambiguity . @testforstephen Please help review this one for organize changes.

@mikoto2000 Please find document for code action carefully: https://microsoft.github.io/language-server-protocol/specification#textDocument_codeAction These two edits & commands are not mutually exclusive.

yaohaizh

Need review from @testforstephen

testforstephen · 2019-06-10T03:29:30Z

....jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java

 		CodeAction codeAction = new CodeAction(CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description);
 		codeAction.setKind(CodeActionKind.SourceOrganizeImports);
-		codeAction.setCommand(command);
+		if (preferenceManager.getClientPreferences().isWorkspaceApplyEditSupported()) {


getOrganizeImportsAction is used for advanced organize imports (Prompt a UI for resolve ambiguous imports). The command it returns is java.action.organizeImports, not java.apply.workspaceEdit. If you don't implement the command in the client side, please keep initializationOptions/extendedClientCapabilities/advancedOrganizeImportsSupport to false.

What you need look at is the function convertToWorkspaceEditAction.

What you need look at is the function convertToWorkspaceEditAction.

Thank you.
I was misunderstanding advanced code action function...

I want to work organize imports, without client-side command implementation.
so I will no fix getOrganizeImportsAction.

I check convertToWorkspaceEditAction and try fix it.

I fixed 7890248 and af1be20.

mikoto2000 · 2019-06-11T23:09:54Z

I fix SourceAssistProcessor#convertToWorkspaceEditAction to use WorkspaceEdit if client supported workspace/applyEdit.

List of support WorkspaceEdit commands:

Organize Imports
Override/Implement Methods
Generate Getter and Setter
Eclipse proposal(e.g. Remove 'unusedField', keep assignments with side effects)

List of no support WorkspaceEdit commands:

Generate hashCode() and equals()
Generate toString()
Generate Constructors
Generate Delegate Methods

Condition of using WorkspaceEdit:

workspace/applyEdit is true
initializationOptions/extendedClientCapabilities/overrideMethodsPromptSupport is false
initializationOptions/extendedClientCapabilities/advancedOrganizeImportsSupport is false
initializationOptions/extendedClientCapabilities/advancedGenerateAccessorsSupport is false

testforstephen

It's better to only add some different test cases, no need for duplicate code

testforstephen · 2019-06-14T09:31:20Z

...sts/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandlerUseWorkspaceEditTest.java

+	}
+
+	@Test
+	public void testCodeAction_exception() throws JavaModelException {


This test case is same as CodeActionHandlerTest. No need of it.

Fixed in da69be9.
(da69be9#diff-40c092ea2f1a4bf03e8de7891cb6625eL125)

testforstephen · 2019-06-14T09:32:49Z

...sts/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandlerUseWorkspaceEditTest.java

+	}
+
+	@Test
+	public void test_noUnnecessaryCodeActions() throws Exception{


Same as CodeActionHandlerTest. No need of it.

Fixed in da69be9.
(da69be9#diff-40c092ea2f1a4bf03e8de7891cb6625eL178)

testforstephen · 2019-06-14T09:33:20Z

...sts/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandlerUseWorkspaceEditTest.java

+		return server.codeAction(params).join();
+	}
+
+	public static Command getCommand(Either<Command, CodeAction> codeAction) {


duplicate with CodeActionHandlerTest.

Fixed in da69be9.

da69be9#diff-40c092ea2f1a4bf03e8de7891cb6625eL166

da69be9#diff-40c092ea2f1a4bf03e8de7891cb6625eL210

testforstephen · 2019-06-14T09:34:22Z

...sts/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandlerUseWorkspaceEditTest.java

+		return $;
+	}
+
+	public static boolean containsKind(List<Either<Command, CodeAction>> codeActions, String kind) {


Duplicated code. Use CodeActionHandlerTest#containsKind directly.

Fixed in da69be9.

I deleted test_noUnnecessaryCodeActions, so no longer needed to call containsKind.

da69be9#diff-40c092ea2f1a4bf03e8de7891cb6625eL202

da69be9#diff-40c092ea2f1a4bf03e8de7891cb6625eL203

da69be9#diff-40c092ea2f1a4bf03e8de7891cb6625eL223

testforstephen

LGTM

testforstephen · 2019-06-19T14:04:37Z

When this PR is merged, it may have some side effects for vscode-java client. Currently vscode-client used a customized applyWorkspaceEdit command to apply workspace edit to client, meanwhile it did formatting to the changed code block because of formatting issue redhat-developer/vscode-java#557.

But when switching to this PR behavior, the vscode will auto apply the workspace edit, but without formatting operation.

mikoto2000 · 2019-06-24T22:32:58Z

But when switching to this PR behavior, the vscode will auto apply the workspace edit, but without formatting operation.

Okay. I reproduced this behavior.

so I want to make format unnecessary after applyWorkspaceEdit.

I propose a solution.

Modify of eclipse.jdt.ls:

Add configurations.

java.format.settings.simple.tabSize
- number of spaces that represents one tabulation
- default is 4
- if java.format.settings.profile or java.format.settings.url is not empty, ignore it.
- (internal behavior): set to DefaultCodeFormatterConstants.FORMATTER_TAB_SIZE of JavaCore option
java.format.settings.simple.tabChar
- tabulation character(tab, space or mixed)
- default is tab
- if java.format.settings.profile or java.format.settings.url is not empty, ignore it.
- (internal behavior): set to DefaultCodeFormatterConstants.FORMATTER_TAB_CHAR of JavaCore option

CodeGenerator respect JavaCore settings so format unnecessary after applyWorkspaceEdit.

processing of vscode-java:

Use those value when initialize and change configuration.

editor.tabSize set to java.format.settings.simple.tabSize
if editor.insertSpaces is true, set space to java.format.settings.simple.tabChar
if editor.insertSpaces is false, set tab to java.format.settings.simple.tabChar

What do you think about it?

…dt.ls#1059)

fbricon · 2019-07-16T17:55:14Z

Sorry but I need to better understand the new behavior before merging it, so I'll punt to target milestone to end of august

gns26 · 2019-09-04T19:53:12Z

Sorry but I need to better understand the new behavior before merging it, so I'll punt to target milestone to end of august

Any updates on this?

fbricon · 2019-09-09T17:38:03Z

Sorry for the delay, I'm stretched on different projects and need to go through my backlog. But I haven't forgotten. I'll review as soon as I can.

fbricon · 2019-10-07T14:35:03Z

Ok, I can look at it now, but after rebasing against master, the code no longer compile.

fbricon · 2019-10-07T14:42:20Z

When this PR is merged, it may have some side effects for vscode-java client. Currently vscode-client used a customized applyWorkspaceEdit command to apply workspace edit to client, meanwhile it did formatting to the changed code block because of formatting issue redhat-developer/vscode-java#557.

But when switching to this PR behavior, the vscode will auto apply the workspace edit, but without formatting operation.

@testforstephen once #1194 and #1157 are fixed, the formatting issue should be fixed right?

testforstephen · 2019-10-08T04:40:58Z

yes, these two issues should be able to fix it.

mikoto2000 · 2019-10-19T11:32:19Z

Ok, I can look at it now, but after rebasing against master, the code no longer compile.

Thank you.
I was able to reproduce the compilation failure.
I am going to check and fix it.

Signed-off-by: mikoto2000 <mikoto2000@gmail.com>

…spaceEdit. Signed-off-by: mikoto2000 <mikoto2000@gmail.com>

Signed-off-by: mikoto2000 <mikoto2000@gmail.com>

mikoto2000 · 2019-12-01T11:56:08Z

I rebased from latest master(d30acc4).

bstaletic · 2019-12-01T12:35:30Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java

 			codeAction.setDiagnostics(context.getDiagnostics());
+			if (useWorkspaceEdit) {
+				codeAction.setEdit(edit);


This can easily throw a NullPointerException. If the proposal is either CUCorrectionCommandProposal or RefactoricCorrectionCommandProposal, there is no WorkspaceEdit created.

This is what happened in #1256 (comment) and was taken care of in #1278. Good luck.

fbricon modified the milestones: Mid May 2019, Mid June 2019 Jun 3, 2019

mikoto2000 force-pushed the use_workspaceedit_on_organizeimports branch from 4c2004b to 03e80cc Compare June 4, 2019 12:27

fbricon requested a review from yaohaizh June 5, 2019 23:32

yaohaizh reviewed Jun 9, 2019

View reviewed changes

testforstephen requested changes Jun 10, 2019

View reviewed changes

testforstephen requested changes Jun 14, 2019

View reviewed changes

mikoto2000 force-pushed the use_workspaceedit_on_organizeimports branch from c7205e2 to da69be9 Compare June 16, 2019 20:33

testforstephen approved these changes Jun 17, 2019

View reviewed changes

shun095 added a commit to shun095/vim-lsp that referenced this pull request Jul 4, 2019

Temporary fix for eclipse.jdt.ls (Waiting for eclipse-jdtls/eclipse.j…

6db08e8

…dt.ls#1059)

fbricon modified the milestones: Mid July 2019, End August 2019 Jul 16, 2019

fbricon removed this from the End August 2019 milestone Sep 2, 2019

jdneo mentioned this pull request Oct 8, 2019

Code actions should return textedits with proper formatting #1157

Closed

bstaletic referenced this pull request in mikoto2000/eclipse.jdt.ls Oct 26, 2019

Modified to use WorkspaceEdit on OrganizeImports.

03e80cc

Signed-off-by: mikoto2000 <mikoto2000@gmail.com>

bstaletic mentioned this pull request Nov 9, 2019

Take advantage of CodeActionLiteral client capability #1256

Merged

testforstephen mentioned this pull request Nov 28, 2019

Take advantage of CodeActionLiteral client capability - Second attempt #1278

Merged

fbricon mentioned this pull request Nov 28, 2019

No delegateCommandHandler for java.apply.workspaceEdit #376

Closed

mikoto2000 added 5 commits December 1, 2019 17:29

Modified to use WorkspaceEdit on OrganizeImports.

07db9a8

Signed-off-by: mikoto2000 <mikoto2000@gmail.com>

Revert change the SourceAssistProcessor#getOrganizeImportsAction method.

918892b

Signed-off-by: mikoto2000 <mikoto2000@gmail.com>

Modified to set WorkspaceEdit to CodeAction if client supporting Work…

24eb0e3

…spaceEdit. Signed-off-by: mikoto2000 <mikoto2000@gmail.com>

Deleted duplicate test codes.

84d093d

Signed-off-by: mikoto2000 <mikoto2000@gmail.com>

Fixed build break.

aef4691

Signed-off-by: mikoto2000 <mikoto2000@gmail.com>

mikoto2000 force-pushed the use_workspaceedit_on_organizeimports branch from da69be9 to aef4691 Compare December 1, 2019 11:26

bstaletic reviewed Dec 1, 2019

View reviewed changes

snjeza mentioned this pull request Jun 17, 2022

No delegateCommandHandler for java.action.organizeImports #2126

Closed

mikoto2000 closed this Oct 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modified to use WorkspaceEdit on OrganizeImports. #1059

Modified to use WorkspaceEdit on OrganizeImports. #1059

mikoto2000 commented Jun 2, 2019

eclipse-ls-bot commented Jun 2, 2019

fbricon commented Jun 2, 2019

fbricon commented Jun 2, 2019

mikoto2000 commented Jun 3, 2019

mikoto2000 commented Jun 4, 2019

yaohaizh commented Jun 9, 2019

yaohaizh left a comment

testforstephen Jun 10, 2019

mikoto2000 Jun 10, 2019

mikoto2000 Jun 11, 2019

mikoto2000 commented Jun 11, 2019

testforstephen left a comment

testforstephen Jun 14, 2019

mikoto2000 Jun 16, 2019

testforstephen Jun 14, 2019

mikoto2000 Jun 16, 2019

testforstephen Jun 14, 2019

mikoto2000 Jun 16, 2019

testforstephen Jun 14, 2019

mikoto2000 Jun 16, 2019

testforstephen left a comment

testforstephen commented Jun 19, 2019

mikoto2000 commented Jun 24, 2019

fbricon commented Jul 16, 2019

gns26 commented Sep 4, 2019

fbricon commented Sep 9, 2019

fbricon commented Oct 7, 2019

fbricon commented Oct 7, 2019

testforstephen commented Oct 8, 2019

mikoto2000 commented Oct 19, 2019

mikoto2000 commented Dec 1, 2019

bstaletic Dec 1, 2019

Modified to use WorkspaceEdit on OrganizeImports. #1059

Modified to use WorkspaceEdit on OrganizeImports. #1059

Conversation

mikoto2000 commented Jun 2, 2019

eclipse-ls-bot commented Jun 2, 2019

fbricon commented Jun 2, 2019

fbricon commented Jun 2, 2019

mikoto2000 commented Jun 3, 2019

mikoto2000 commented Jun 4, 2019

yaohaizh commented Jun 9, 2019

yaohaizh left a 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

mikoto2000 commented Jun 11, 2019

testforstephen left a 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

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

testforstephen left a comment

Choose a reason for hiding this comment

testforstephen commented Jun 19, 2019

mikoto2000 commented Jun 24, 2019

fbricon commented Jul 16, 2019

gns26 commented Sep 4, 2019

fbricon commented Sep 9, 2019

fbricon commented Oct 7, 2019

fbricon commented Oct 7, 2019

testforstephen commented Oct 8, 2019

mikoto2000 commented Oct 19, 2019

mikoto2000 commented Dec 1, 2019

Choose a reason for hiding this comment