bug#71642: 30.0.50; eglot-execute doesn't support ExecuteCommandParams

[Troy Brown <brownts@troybrown.dev>]

[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]

On Thu, Jun 20, 2024 at 5:46 AM João Távora <joaotavora@gmail.com> wrote:
>
> On closer analysis the only confusing thing is that the spec
> encourages clients to call textDocument/executeCommand with
> illegal ExecuteCommandParams objects that contain an extraneous
> 'title'
>
> Most servers apparently don't care, but Eglot is doing this and
> it's a subtle bug.
>
> I think the right thing to do is to have Command _and_
> ExecuteCommandParams in the matcher.
>

Thanks for the explanation.

> I've done a tweak to your patch.  See after my sig.  It's untested, so
> please have a look. If you can, test, add more comments, change the order
> if indeed it's needed.  You can also change the implementation if you think
> the recursiveness makes it more confusing.
>

I've updated the patch based on your suggestions.  I've tested all
invocations of executeCommand through Command, ExecuteCommandParams
and CodeAction and they all appear to be working as expected now.  I
think the recursion is fine, it's simple and elegant.  With regards to
the order, you are correct, since the command in CodeAction isn't a
string, it's not a problem.  I was previously erroneously only testing
with a string.

[-- Attachment #2: 0001-Eglot-Fix-command-execution-bug-71642.patch --]
[-- Type: text/x-patch, Size: 2504 bytes --]

From 963e5c975f89d9c6111902694ac51facaf7fc2b8 Mon Sep 17 00:00:00 2001
From: Troy Brown <brownts@troybrown.dev>
Date: Wed, 19 Jun 2024 20:14:07 -0400
Subject: [PATCH] Eglot: Fix command execution (bug#71642)

* lisp/progmodes/eglot.el (eglot--lsp-interface-alist): Add
ExecuteCommandParams interface.
(eglot--execute): Fix handling of Command and CodeAction and add
ExecuteCommandParams.

Copyright-paperwork-exempt: yes
---
 lisp/progmodes/eglot.el | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 03da5c7b22a..df4cbe50dc0 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -624,6 +624,7 @@ eglot--uri-path-allowed-chars
                              :command :data :tags))
       (Diagnostic (:range :message) (:severity :code :source :relatedInformation :codeDescription :tags))
       (DocumentHighlight (:range) (:kind))
+      (ExecuteCommandParams ((:command . string)) (:arguments))
       (FileSystemWatcher (:globPattern) (:kind))
       (Hover (:contents) (:range))
       (InitializeResult (:capabilities) (:serverInfo))
@@ -884,17 +885,25 @@ eglot-execute-command
 
 (cl-defgeneric eglot-execute (server action)
   "Ask SERVER to execute ACTION.
-ACTION is an LSP object of either `CodeAction' or `Command' type."
+ACTION is an LSP `CodeAction', `Command' or `ExecuteCommandParams'
+object."
   (:method
    (server action) "Default implementation."
    (eglot--dcase action
-     (((Command)) (eglot--request server :workspace/executeCommand action))
+     (((Command))
+      ;; Convert to ExecuteCommandParams and recurse (bug#71642)
+      (cl-remf action :title)
+      (eglot-execute server action))
+     (((ExecuteCommandParams))
+      (eglot--request server :workspace/executeCommand action))
      (((CodeAction) edit command data)
       (if (and (null edit) (null command) data
                (eglot-server-capable :codeActionProvider :resolveProvider))
           (eglot-execute server (eglot--request server :codeAction/resolve action))
         (when edit (eglot--apply-workspace-edit edit this-command))
-        (when command (eglot--request server :workspace/executeCommand command)))))))
+        (when command
+          ;; Recursive call with what must be a Command object (bug#71642)
+          (eglot-execute server command)))))))
 
 (cl-defgeneric eglot-initialization-options (server)
   "JSON object to send under `initializationOptions'."
-- 
2.37.1