Fix duplicate client/registerCapability for workspace/executeCommand

[angelozerr]

Fix duplicate client/registerCapability for workspace/executeCommand

Fixes #937

Signed-off-by: azerr [email protected]

[deathaxe]

OK, downloaded OpenJDK 15, pulled this PR, ran mvn initialize and mvn clean verify.

The build process fails with the following error.

As it is the first time trying to compile Java stuff, I am a bit lost at this point.

[ERROR] Failed to execute goal org.apache.felix:maven-bundle-plugin:4.1.0:manifest (bundle-manifest) on project org.eclipse.lemminx: Execution bundle-manifest of goal org.apache.felix:maven-bundle-plugin:4.1.0:manifest failed.: ConcurrentModificationException -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <args> -rf :org.eclipse.lemminx

[angelozerr]

That's strange, our CI build is working but we compile it with JDK 8.

[deathaxe]

I managed to build with OpenJDK 11.

Your changes appear to work correctly.

I see only one LSP: LemMinX: registering capability: executeCommandProvider message without any warnings and the registration messages look normal, too.

:: <-- LemMinX client/registerCapability(12): {'registrations': [{'method': 'workspace/executeCommand', 'registerOptions': {'commands': ['xml.validation.current.file', 'xml.validation.all.files'], 'id': '4831061d-bb0c-4397-8cf0-d956ed89b982'}, 'id': '4831061d-bb0c-4397-8cf0-d956ed89b982'}]}
:: >>> LemMinX 12: None

Running the commands works as well.

1. Validate all files:

   ST console command

   view.run_command("lsp_execute", {"command_name": "xml.validation.all.files"})
   

   Message log

   :: --> LemMinX workspace/executeCommand(2): {'command': 'xml.validation.all.files'}
   :: <-  LemMinX textDocument/publishDiagnostics: {'diagnostics': [], 'uri': 'file:///C:/Apps/Sublime%20Text/Data/Packages/LSP-lemminx/schemas/sublime-snippet.xsd'}
   :: <<< LemMinX 2: None
   

2. Validate a single file:

   ST console command

   view.run_command("lsp_execute", {"command_name": "xml.validation.current.file", "command_args": [{"uri": "file:///C:/Apps/Sublime%20Text/Data/Packages/LSP-lemminx/schemas/sublime-snippet.xsd"}]})
   

   Message log

   :: --> LemMinX workspace/executeCommand(4): {'arguments': [{'uri': 'file:///C:/Apps/Sublime%20Text/Data/Packages/LSP-lemminx/schemas/sublime-snippet.xsd'}], 'command': 'xml.validation.current.file'}
   :: <-  LemMinX textDocument/publishDiagnostics: {'diagnostics': [], 'uri': 'file:///C:/Apps/Sublime%20Text/Data/Packages/LSP-lemminx/schemas/sublime-snippet.xsd'}
   :: <<< LemMinX 4: None
   

[BoykoAlex]

Seems like with this change all commands would be registered/unregistered against one registration id which is probably ok. Our Spring XML extension should work fine with this change... Wonder if we need to call unregister in CapabilityManager with the constant registration id before calling register...

[BoykoAlex]

We don't seem to call unregisterCapability anywhere... if commands are registered again does this "registration over registration" works ok in VScode and Eclipse?

[deathaxe]

Seems like with this change all commands would be registered/unregistered against one registration id which is probably ok.

You should keep in mind the mechanism is to register language server capabilities which is workspace/executeCommand but not the list of available arguments it supports - what you call commands.

A client can store a list of commands exposed via this registration handshake if needed.

[datho7561]

If I understand correctly, in order to support adding and removing individual commands after the first initialization, we could use the following process each time we modify the set of commands:

1. Unregister the workspace/executeCommand capability, which unregisters all the old commands with the client
2. Register a new workspace/executeCommand capability with the new set of commands

I'll see if I can implement this.

[datho7561]

As of right now we have no use case for unregistering commands, so this solution works. @deathaxe does sublime-lsp support registering new commands dynamically by sending another registration for workspace/executeCommand that just contains the new command?

[deathaxe]

It just overwrites the former registration and puts a debug message to console about it happening.

The list of registered commands is not very interesting as it is lacking required information to build menu items or command palette entries anyway. It just helps to see what the language server has to offer in general.

Finally it's just interesting to see workspace/executeCommand being supported.

Command Palette entries are defined statically in local files, which is no problem as LemMinX is handled by dedicated LSP-lemminx helper package.

[angelozerr]

Thanks @datho7561 @BoykoAlex for your review