Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: lsp--find-workspaces-for workspace/executeCommand respects the target command #4494

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kassick
Copy link
Contributor

@kassick kassick commented Jul 11, 2024

Calling

(lsp--find-workspaces-for '(:method "workspace/executeCommand"
                            :params (:command "arbitrary-command")))

would return any workspace that had the :executeCommandProvider capability registered.

This would result in a given command being forwarded for several active workspaces that wouldn't know how to handle that command. Some servers (such as ruff) would raise a KeyError for unknown commands -- causing the minibuffer to, sometimes, show an error.

This change introduces a new :check-message callback that can be used in lsp-method-requirements -- this callback receives the workspace and the message to be sent.

Ths entry for workspace/executeCommand has been updated to check if the target workspace can execute the required command.

…arget command

Calling

    (lsp--find-workspaces-for '(:method "workspace/executeCommand"
                                :params (:command "arbitrary-command")))

would return *any* workspace that had the :executeCommandProvider capability
registerd.

This would result in a given command being forwarded for **all active
workspaces**. Some servers (such as ruff) would raise a KeyError for unknown
commands -- causing the minibuffer to, sometimes, show an error.

This change introduces a new :check-message callback that can be used in
lsp-method-requirements -- this callback receives the workspace and the
message to be sent.

Ths entry for workspace/executeCommand has been updated to check if the target
workspace can execute the required command.
@kassick
Copy link
Contributor Author

kassick commented Aug 20, 2024

Do you folks think this is the right approach to avoid sending commands that a server will reject? Any suggestions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant