Clarify the intension of how commands are defined

[kdvolder]

There seems to be a great deal of confusion around the mechanism by which commands are defined.

Personally, my take on it is that commands 'exist' somehow on the client-side. I.e. there is client-side command registry. Any command that exists is somehow or other entered into this registry.

This registry functions like a global namespace into which every existing command irrespective of how it is/was defined, is entered.

So I imagine that, allthough the spec doesn't currently have standard list of commands the idea is that at some point in the future, a standardized list of commands might/will/should be created and added to the spec. And I assume this is the direction in which this part of the spec is intended to evolve.

I am also aware that there is now a registerCapability request that allows a server to explicitly request specific command ids be delegated to the server so that a server may implement these commands.

By my interpretation, this does not change the fact that ultimately, the command is still a client-side thing.
I.e. commands defined in this way still primarily exist as 'executable entities' on the client side (i.e. they are entered into the same global command registry as any other command that comes 'baked in' with the client). The client simply happens to implement these commands by delegating there execution to the server.

I think my interpretation is in-line with how this works in vscode. And so I feel confident this is the intended interpretation of the spec. But there are some who disagree with this and suggest that I may not be reading the spec right. (I must admit, a lot of my interpretation is read between the lines, and though it all seems logical to me, the spec doesn't say any of this very clearly, and leaves a lot of that wide open to interpretation).

For an example of the kinds of confusion that seems to be going on take a look at this:

atom/atom-languageclient#183

The folks there seem to lean more towards an interpretation that there is no 'central command registry', but rather a command registry is 'private' to a single client <-> server connection. So that, for example any command requested to be executed by a given server (say by including it in code action) must be defined by that same server (so not, for example a 'baked in to the client' or 'defined by another server' command).

Can we please get some clarity on this? If not in the spec, at least some discussion here to try and reach some kind of consensus?

[kdvolder]

BTW: It might help focus the discussion to highlight the very words in the spec that seem to be responsible for causing the confusion. I.e the very words that seem to oppose my interpretation. These are the words:

The code action request is sent from the client to the server to compute commands for a given text document and range. These commands are typically code fixes to either fix problems or to beautify/refactor code. The result of a textDocument/codeAction request is an array of Command literals which are typically presented in the user interface. When the command is selected the server should be contacted again (via the workspace/executeCommand) request to execute the command.

If we read those words there is very little room for interpretation as it literally says that 'the server should be contacted again' to execute the command. There is no 'if', 'what' or 'but' here.

So my question really boils down to this. Is that really intentional? Is it really the case that all commands, not subject to any conditions, should allways be send back to the server for execution?

[kdvolder]

It has been 14 days. Can someone please provide some feedback here? My take on in it is that the highlighted words I quoted from the spec were unintentially phrased a bit too strongly and it is leading the atom folks to purposefully implement it exactly as worded which explicitly different from how the vscode client works.

I may be wrong about my opinion on 'the true intent' of whoever wrote those phrases, but I'd really like to know either way.

The current confision does not serve anybody and will lead to situation were language server authors will be forced to make choices to implement functionalities that only work on atom and not vscode, or vice versa, or add special case logic to the server to implement the same functionality twice depending which client is being targeted.

[dbaeumer]

@kdvolder I apologize for the delay in my response. Was out on vacation and then busy with the 1.22 release.

I agree with your statement that the command behavior is under-specified and especially causes confusion with code actions, code completion since it is easily arguable that for a code action there doesn't even need to be a global command registry. I would like to make this more clear in the following way:

• specify indeed that commands are global since it does make sense to invoke them
• allow the code action request to return an literal that already contains the workspace manipulations (so to speak a local command).
• for code complete this is already possible since a completion item has text edit support. If a completion item needs to manipulate other resources then we need to extend the completion item

Would this be something you think goes into the right direction?

[kdvolder]

@dbaeumer Sorry... I somehow missed your response. Too many emails cluttering inbox.

But yes. Your suggestions make sense.

One thing that I'm specifically concerned about / interested, which you do not mention.... is to have clarity around scenarios involving multiple servers.

For example let's say Server A registers a command called 'doA'
And Server B returns a code action which references command 'doA'.

If the command registry is truly a global name space on the client-side, then it would mean the client should know to route command execution from a code action returned by server B to be executed on Server A (because server A registered that command).

I think this is also the kind of scenario the discussion in the referenced atom issue was about (i.e. one server wants to return code actions where the actual implementation of the command actually is contributed from another server).

I don't have a opinion the specifics around completion items. Other than perhaps... that I would hope the mechanic for executing commands would not be different depending on whether the command reference is returned as part of a code action, a completion item or... whatever.

I.e. I think the meaning and execution of a given command id should be handled in exactly the same way in all situations where a command id is passed from a server to a client.

[kdvolder]

I realize this could lead to long discussions around commands and their execution, for which., perhaps LSP is not yet ready to nail down every detail into the spec.

So... at the very least we should try to somehow fix the hihlighted text quoted in #432 (comment)

I propose that it be made less strong by not saying things like 'should be contacted again'.

It makes actually little sense to me to have a mechanism for a server to be able to declare that it is capable of handling specific commands and then not actually make use of it in this instance. Since there is a list of commands the server can handle... the wording should probably be made more 'conditional' and say something like:

if the server has registered a capability to handle the command, it should be contacted again.

You could leave it still open to interpretation what should happen in if the server did not register capability to handle a command. But at least I think this would remove the one phrase that seems to cause most confusion from the spec.

If you want to make it more strong towards implying the existence of a global command registry then the words could become:

if any server has registered a capability to handle the command, then that server should be contacted to execute the command.

The use of the word 'any' implies that even for commands registered by other servers this rule applies.

The execution of commands not registered by any servers is left open to interpretation. I think that is fine.

[dbaeumer]

@kdvolder I agree with your view. The specification shouldn't mention anything about a command registry but should only talk about that if a server returns a command id and registers a handler for the same id that handler should be called independent of the number of server instances. I will clean up the specification and change the implementation in the vscode node modules to handle this properly.

[kdvolder]

change the implementation in the vscode node modules to handle this properly

Uh, actually, now you make me worry. As far as I can tell the implementation is correct / good already. So what are you thinking of changing exactly? I am asking because we actually have some functionality that depends on the ability of one server to register a command and another server or vscode extension to trigger execution of that command. So I hope you are not thinking of changing it in such a way that it would make that kind of thing no longer possible.

[dbaeumer]

Actually that was the idea. If we want to make the use case described by you always work then we need to spec that something like a global command exists and how it should work. Your proposal:

if any server has registered a capability to handle the command, then that server should be contacted to execute the command.

pointed into that direction for me.

What exactly is the use case where you trigger a command from server A but the execution happens in server B. May be we find a different solution.

[kdvolder]

The use case is to support information requests and callbacks from one language server to another.

Our specific case is that of attaching a classpath listener to JDT.LS from our spring-boot language server.

JDT.LS keeps track of projects and classpaths. Our language server wants to receive information about projects and classpaths on a continuing basis (i.e. like subscribing a listener to a model). We have added an extenions to JDT.LS that offers this kind of a service via command 'jdt.ls.addClasspathListener'. We pass a command id to that command as a parameter. Our language server registers the command. When there is something interesting to communicate (project classpath changed). The 'callback' command is exeucted with classpath information as parameter.

To make this work we also needed to add a custom protocol extentsion on the client-side for jdt.ls to allow server to execute commands on the client (i.e. like `workspace/executeCommand' but being sent from server to client). I had intented to propose this as a protocol extension for LSP, but haven't gotten round to starting that particular ball rolling yet.)

To be honest, the whole mechanic feels very complex and convoluted. So if there is a better way I'd be all for it.

Another use case, which you will probably agree is rather important, is the way Java debug extension in vscode obtains classpath information for launching Java processes. The Java debug extension does this by adding a command (via same extension mechanism as we use) to JDT that allows the debugger to ask JDT.LS for classpath information.

This is similar to what we do. (We took inspiration from them). Except the debugger has simpler needs. It doesn't need a callback. Just a 'ask for classpath now' type of request. But both rely on the 'central command registry' for the calls into language server from another extension / server to work.

I.e. a 'getClasspathInfo' command is register by JDT.LS. This command is then called by the debugger (which is a separate extension) to get the classpath information it uses to launch/debug a Java process.

[dbaeumer]

I was not aware of this. IMO something like this should be more explicit to allow this kind of server to server communication for clients which don't have such a global registry and might not be able to implement one.

What we should do is to extend the VS Code client to handle the command execution for special commands and then let the extension forward the request to the right server instead of letting VS Code doing it.

In the case you described are all servers / debugger adpaters managed by the same extension. Or are they different.

@aeschli are you aware of this?

[kdvolder]

What we should do is to extend the VS Code client to handle the command execution for special commands and then let the extension forward the request to the right server instead of letting VS Code doing it.

That sounds good, but wouldn't that mean all servers must then be handled by the same client instance? Right now a typical vscode extension has its own instance of the vscode client as a nodejs dependency. I suppose it is probably possible for multiple vscode client instances to somehow share state with eachother to store the commands (but aren't we then back to where we started from... i.e. a shared command registry :-).

Secondly, I doubdt the JDT debugger extension even uses the vscode lsp client at all, since its not even an LSP server (I think it implements debugger protocol, which is separate/different from the LSP, is it not?).

In the case you described are all servers / debugger adpaters managed by the same extension. Or are they different.

Different. The code for the different extension / servers isn't even owned by the same people. (I.e. we are trying to communicate with a language server / extension written by someone other than ourselves).