client/executeCommand

[kdvolder]

See: #1117

[rwols]

I get that you're trying get something out the door but this needs many more options for clients. Given just this state of the PR a server doesn't know whether a command can succeed or not.

Just to give you some ideas:

• the client should be able to communicate dynamically to the server about possible new commands that it knows about, similar to workspace/didChangeConfiguration.
• It should be possible for the server to get a list of known command names from the client, similar to workspace/configuration

At this point the server can only do client/executeCommand and hope for the best.

[mickaelistria]

I don't think we should require more options at this point. The current proposal is good and there is room for evolution (such as listing commands) in the future. However, we don't really have a strong need to pass a static list of commands at that point, just like symmetrically, there is no strong need for clients to know in advance the list of server commands it can invoke.
The command pattern is often use for client or LS specific integrations, that we cannot really standardize in LSP because they're not generic enough.
I think it's better to mimic the current workspace/executeCommand action (and related capabilities), it seems more consistent.

At this point the server can only do client/executeCommand and hope for the best.

It's the same for workspace/executeCommand, and yet this is working well ;)

[rwols]

It's the same for workspace/executeCommand, and yet this is working well ;)

Not quite. If you're lucky, you can read a readme file explaining what the arguments of a command can be. If you're unlucky, you have to dig through the server source code to figure out what a command can do.

[rwols]

The configure-less way of invoking commands is via code actions, which works well, I agree.

[kdvolder]

I'm not against adding more options in the future for things such as 'command discovery' or making the spec more precise about mechanism by which commands are made to 'exist' on the client side.

However, I wanted to explicitly avoid this becoming yet another ticket dragged out into a lengthy discussion with no actionable outcome.

To me these questions are somewhat orthogonal:

1. how does a server request a client to execute a command
2. how does a server know that a client can execute a specific (set of) commands
3. how are client-side commands defined

So I explicitly wanted limit the scope of this ticket to be just about 1. The other questions can be answered independently and in fact there are already some tickets open with long discussions.

Given just this state of the PR a server doesn't know whether a command can succeed or not.

That is true, but at least we know how to request execution in a standardised way. I.e. we are answering question 1. So to me that is progress.

And we can assume an error comes back if the execution failed (I agree with your other comments that we can perhaps make some of the error cases more explicit in the spec).

[rwols]

No worries. I am just some guy maintaining a client and weary of opening pandora's box with client/executeCommand. We must have a follow-up PR that defines how command discovery works, and we must standardize some "common" commands.

[kdvolder]

I don't really disagree with you. I just don't want to go there yet. Yes, I could imagine some options added to capability object in the future such as 'commands: [...]' for a list of 'statically' known commands. Or a property 'commandDisovery: boolean' that indicates there is some extra protocol for command discovery. etc. But this sort of thing could be arbitrarily complex and with lots of people wanting something a little different from it. So I just don't think we should/can tackle all of these at once right now. The more we can separate this into smaller questions/issues, the more chance we have of actually making progress in small increments.

[rwols]

If this ultimately also leads to standardizing commands like "run auto-complete" and "commit completion" in a client-neutral way then I'm okay with this.

[mickaelistria]

If this ultimately also leads to standardizing commands like "run auto-complete" and "commit completion" in a client-neutral way then I'm okay with this.

I don't get the use-case here, the client is supposed to orchestrate that directly with LSP operations, why would command be useful for that?
If we can identify some generally useful server side commands that we're ready to spec, then it's better to just specify them as proper LSP operation, with proper parameters definition and so on than leaving it in a pot-pourri of command ids without finer specification.

[rwols]

I don't get the use-case here, the client is supposed to orchestrate that directly with LSP operations, why would command be useful for that?

There are servers that put "command": "editor.action.triggerSuggest" in some of their completion items. This means "show the auto-complete widget". But it's not standardized.

If we can identify some generally useful server side commands that we're ready to spec, then it's better to just specify them as proper LSP operation, with proper parameters definition and so on than leaving it in a pot-pourri of command ids without finer specification.

For sure :)

[dbaeumer]

@kdvolder thanks for the PR. Besides updating the specification we need a reference implementation somewhere as well. This can be VS Code, Eclipse or any other client. Is someone willing to do that as well?

[kdvolder]

This can be VS Code, Eclipse or any other client. Is someone willing to do that as well?

I was planning on doing it yes, just need to find the time :-)

[dbaeumer]

@kdvolder great.

[radeksimko]

I appreciate the work being done here and aim to ship something.

Do you mind describing some concrete use cases this is aiming to solve?

I ran into at least two use cases myself where I had the need to execute client-side commands:

• open completion prompt
• show references

In both cases these are commands that would benefit from being linked directly from their relevant methods - i.e. CompletionItem.command or CodeLens.command.

Executing a separate standalone method would require either extra roundtrip where the server implements some kind of a proxy server-side command for any client-side command, or somehow entirely bypass the existing workflow.

Are there some other use cases this is meant to address?

---

I agree with @rwols here that for server to not know what commands are available on the client it makes it practically impossible to provide a decent UX in cases I outlined above. Imagine that server implements code lens to show reference counts, user clicks to "5 references", linking to editor.action.showReferences and the client passes it back to the server because it doesn't implement it. The server can't do anything about it so the click ends up as no-op in the UI. That doesn't seem ideal? It would seem better if the server could just suppress the code lens entirely and avoid showing the link in the first place.

[radeksimko]

I just found a use case for this feature. I assume it would be useful for updating some UI elements when the server deems it necessary.

The client may pull certain details (such as language/dependency versions or constraints, or some sort of hierarchy) about the codebase via custom command, but these details may get outdated as the user works on the codebase. It would therefore be useful for the server to notify the client when such details change.

I suppose that is the reason why these refresh commands exist anyway

• workspace/codeLens/refresh
• workspace/semanticTokens/refresh

[dbaeumer]

Any progress on this PR?