Take advantage of CodeActionLiteral client capability - Second attempt

[bstaletic]

Another attempt at solving #376. Github seems confused about the state of #1256, so I decided to open a new one.

Changes compared to #1256:

• The error reported by @fbricon should be fixed now.
• There's no more isSupportedCodeActionLiteral() because we should always be checking the code action kind.
  • In both places where isSupportedCodeActionLiteral() was used in Take advantage of CodeActionLiteral client capability #1256, the isSupportedCodeActionKind(kind) was checked a few lines above, so the actual functional changes of the PR could be slightly simplified (compared to Take advantage of CodeActionLiteral client capability #1256).
• Correctly handling code action literals meant that there's a lot of places where the code basically does:
  • Pull Command out of Either<Command, CodeAction>.
  • If that Command is not null to thing A.
  • Else do thing B.

Fixes #376

[eclipse-ls-bot]

Can one of the admins verify this patch?

[fbricon]

test this please

[bstaletic]

@fbricon The tests are already there.

[fbricon]

@bstaletic I know, that comment was a trigger for Jenkins to run a build ;-)

[bstaletic]

Oh... I thought that was addressed to me, because the last time I saw that comment I really did forget to commit the tests. :)

[fbricon]

@bstaletic thanks for your contribution!

[testforstephen]

This PR will always return a WorkspaceEdit instead of command, that brings the same formatting issue as the previous PR #1059.

[bstaletic]

@testforstephen Not always, just when the command that would be returned is already a WorkspaceEdit. If I understand #1059 (comment) correctly, the formatting issue that you have pointed out is completely on the client side?

[testforstephen]

Not a client issue, the root cause is jdt.ls generates the WorkspaceEdit without regarding the actual indentation used by the client. The fix on client side is a workaround to force reformatting the new changes during applying workspace edit.

[bstaletic]

Alright, I get it now. The workaround in the vscode client (obviously) doesn't exist in other clients (like ycmd). Ycmd simply ignores the wrong indentation and lets the users deal with it. So not all clients will notice the regression.

That said, I do see the problem. However, I have no idea where to even start looking for a solution to that bug.

I do see a way to retain the usefulness of the custom applyWorkspaceEdit command handler in vscode-java. Instead of returning a CodeAction literal, using the edit property, how about implementing a handler as expected by clients in #376. The clients would be able to make an executeCommand request and JDT would handle it similarly to the way it handles executeCommand request for oranizeImports command. Compared to the current solution:

• It won't be as efficient as a code action literal, since more requests need to be made.
• Even clients that don't support code action literals will be able to resolve this code action in a generic way.

[fbricon]

Since vscode is by far the largest consumer of jdt.ls, I think introducing that formatting regression would be detrimental to a too large number of users.
So I'll revert the fix until we can come up with a solution that is acceptable for everybody.

[jnturton]

@bstaletic I'm interested in maintaining a very basic fork of this language server that supports the use of workspace/executeCommand by clients, specifically kak-lsp. I looked at the modifications in your PR here overlaid on the eclipse.jdt.ls 1.19 source but the affected classes have evolved since then. Before I wade in further which will require me developing more code level understanding than I currently have, would you be interested in rebasing this PR on the master branch moving it here (or to a fork of your own)?