None of the code actions take effect except "Dismiss: ..."

[sychen52]

lsp-grammarly works fine, though. Do you have any idea what could be the cause?

[sychen52]

I also tried clangd, and its code actions work fine, so it seems to be grammarly related.

[jcs090218]

No, I haven't really tried this package yet. I think it should related to the language server (upstream); maybe related to https://github.com/znck/grammarly/issues/277.

Edit: Before server support workspace/executeCommand, everything work but the Dismiss: .... Sounds like a hint!?

[sychen52]

Here is the issue I find. In eglot-code-actions function: it gets an action from the server first. Then it handles two types of action: Command or CodeAction. The one grammarly lsp send back is similar to CodeAction. However, :edit and :command fields are missing. The following is the definition of a CodeAction and the action returned by grammarly lsp: @jcs090218, Do you have any suggestions for further investigation?

      (CodeAction (:title) (:kind :diagnostics :edit :command :isPreferred))

(:title "Correct your spelling — overall" :kind "quickfix" :diagnostics
        [(:data "_215" :message "Correct your spelling" :range
                (:start
                 (:line 9 :character 50)
                 :end
                 (:line 9 :character 58))
                :source "Grammarly" :severity 1)]
        :data
        (:uri "file:///........org" :suggestionId "_215" :replacementId 0))

[jcs090218]

Sorry it took me a while to respond to this issue. Was busy with something else!

My suggestion is to compare the two packets (or messages) from the two Emacs client (one from lsp-mode, and one from eglot). It seems like there are some implementation differences between the two lsp client, so their responds or actions are different.

Maybe @joaotavora could help us? 😕

[ssl19]

It seems related to eglot's lack of support for codeAction/resolve.

[ssl19]

(defun eglot--read-execute-code-action@override (actions server &optional action-kind)
  "Support for codeAction/resolve."
  (let* ((menu-items
          (or (cl-loop for a in actions
                    collect (cons (plist-get a :title) a))
              (apply #'eglot--error
                     (if action-kind `("No \"%s\" code actions here" ,action-kind)
                       `("No code actions here")))))
         (preferred-action (cl-find-if
                            (lambda (menu-item)
                              (plist-get (cdr menu-item) :isPreferred))
                            menu-items))
         (default-action (car (or preferred-action (car menu-items))))
         (chosen (if (and action-kind (null (cadr menu-items)))
                     (cdr (car menu-items))
                   (if (listp last-nonmenu-event)
                       (x-popup-menu last-nonmenu-event `("Eglot code actions:"
                                                          ("dummy" ,@menu-items)))
                     (cdr (assoc (completing-read
                                  (format "[eglot] Pick an action (default %s): "
                                          default-action)
                                  menu-items nil t nil nil default-action)
                                 menu-items))))))
    (cl-labels ((apply-code-action (chosen first-p)
                  (eglot--dcase chosen
                    (((Command) command arguments)
                     (eglot-execute-command server (intern command) arguments))
                    (((CodeAction) edit command)
                     (when edit (eglot--apply-workspace-edit edit))
                     (when command
                       (eglot--dbind ((Command) command arguments) command
                         (eglot-execute-command server (intern command) arguments)))
                     (when (and (eglot--server-capable :codeActionProvider
                                                       :resolveProvider)
                                first-p)
                       (apply-code-action (eglot--request server :codeAction/resolve chosen) nil))))))
      (apply-code-action chosen t))))

(advice-add #'eglot--read-execute-code-action :override #'eglot--read-execute-code-action@override)

You could try code actions of eglot-grammarly with this hack :)

[krisbalintona]

@ssl19 Wow! Awesome that you not only figured out the source of the missing functionality, but you also wrote a working fix. Thanks!

Have you considered contributing this to Eglot, since the missing functionality is with Eglot itself?

[joaotavora]

This patch won't apply currently, and I don't understand exactly how the last part of the logic works.

Anyway this isn't so much related to "eglot's lack of support for codeAction/resolve." as it is to this LSP server's nonobservance of the standard, which says that you shouldn't rely on a feature that the other endpoint hasn't declared support for. And Eglot indeed doesn't support codeAction/resolve for now. I'll see what I can do...

[joaotavora]

Can someone using a server with these capabilities try out this patch to Eglot?

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index f09c348143d..5a716cd6aef 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -463,7 +463,7 @@ eglot--accepted-formats
 (eval-and-compile
   (defvar eglot--lsp-interface-alist
     `(
-      (CodeAction (:title) (:kind :diagnostics :edit :command :isPreferred))
+      (CodeAction (:title) (:kind :diagnostics :edit :command :isPreferred :data))
       (ConfigurationItem () (:scopeUri :section))
       (Command ((:title . string) (:command . string)) (:arguments))
       (CompletionItem (:label)
@@ -739,9 +739,12 @@ eglot-execute
    (server action) "Default implementation."
    (eglot--dcase action
      (((Command)) (eglot--request server :workspace/executeCommand action))
-     (((CodeAction) edit command)
-      (when edit (eglot--apply-workspace-edit edit))
-      (when command (eglot--request server :workspace/executeCommand command))))))
+     (((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))
+        (when command (eglot--request server :workspace/executeCommand command)))))))

 (cl-defgeneric eglot-initialization-options (server)
   "JSON object to send under `initializationOptions'."
@@ -825,6 +828,7 @@ eglot-client-capabilities
              :documentHighlight  `(:dynamicRegistration :json-false)
              :codeAction         (list
                                   :dynamicRegistration :json-false
+                                  :resolveSupport t :dataSupport t
                                   :codeActionLiteralSupport
                                   '(:codeActionKind
                                     (:valueSet

[krisbalintona]

@joaotavora You're right about emacs-grammarly's non-observance of the standard. I believe the package is maintained by only one person?

Anyway, from my testing, the patch you shared results in identical functionality to the @ssl19's code (according to my very limited "testing"). (Also, thank you for your contributions to Eglot and Emacs)

[joaotavora]

OK. In that case I'll push the patch to master. It should appear in the next Eglot release 1.16.

On an unrelated note, I've also noticed that eglot-grammarly is and extremely thin customization that -- in my humble opinion -- didn't need to exist.

It would seem users can get the same effect as this package with this single form:

(add-to-list 
  'eglot-server-programs
    `((text-mode latex-mode org-mode markdown-mode) "grammarly-languageserver" "--stdio" 
           :initializationOptions (:clientId "client_BaDkMgx4X19X9UxxYRCXZo"))

This is untested though, as I don't have grammarly-languageserver

[joaotavora]

OK. In that case I'll push the patch to master. It should appear in the next Eglot release 1.16.

Done. See https://git.savannah.gnu.org/cgit/emacs.git/commit/lisp/progmodes/eglot.el?id=3b7273f4ae3623962c5d5fdc922a62af1136f448

[jcs090218]

@joaotavora You're right about emacs-grammarly's non-observance of the standard. I believe the package is maintained by only one person?

Yeah, the entire org is maintained by me. :)

On an unrelated note, I've also noticed that eglot-grammarly is and extremely thin customization that -- in my humble opinion -- didn't need to exist.

I don't think I can change your mind, but I am going to put up my reasons why this is needed:

1. Save startup time (Yeah, it's stupid, but it's important to me since I have used up 500+ packages in my config)
2. A place (this repo) for me to track changes from the language server (grammarly)
3. I think it's stupid to paste a code snippet directly to your config (in this case). What something has changed from the upstream? Why not let the other (the community) help you keep track of things?

---

I believe the issue can be closed. Thanks to everyone who participates in this issue!

[joaotavora]

I think it's stupid to paste a code snippet directly to your config (in this case). What something has changed from the upstream? Why not let the other (the community) help you keep track of things?

Stupid? A one-liner? Could even be in Eglot upstream! And setting up a package doesn't need any config? And keeping it up to date? Are you sure you're not busy looking for an excuse to make a package, any package? Then why not make an useful one?

[chaosite]

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index f09c348143d..5a716cd6aef 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -463,7 +463,7 @@ eglot--accepted-formats
 (eval-and-compile
   (defvar eglot--lsp-interface-alist
     `(
-      (CodeAction (:title) (:kind :diagnostics :edit :command :isPreferred))
+      (CodeAction (:title) (:kind :diagnostics :edit :command :isPreferred :data))
       (ConfigurationItem () (:scopeUri :section))
       (Command ((:title . string) (:command . string)) (:arguments))
       (CompletionItem (:label)
@@ -739,9 +739,12 @@ eglot-execute
    (server action) "Default implementation."
    (eglot--dcase action
      (((Command)) (eglot--request server :workspace/executeCommand action))
-     (((CodeAction) edit command)
-      (when edit (eglot--apply-workspace-edit edit))
-      (when command (eglot--request server :workspace/executeCommand command))))))
+     (((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))
+        (when command (eglot--request server :workspace/executeCommand command)))))))

 (cl-defgeneric eglot-initialization-options (server)
   "JSON object to send under `initializationOptions'."
@@ -825,6 +828,7 @@ eglot-client-capabilities
              :documentHighlight  `(:dynamicRegistration :json-false)
              :codeAction         (list
                                   :dynamicRegistration :json-false
+                                  :resolveSupport t :dataSupport t

This breaks Eglot for (at least) rust-analyzer, since it doesn't match the specification of codeAction.resolveSupport: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_codeAction

[joaotavora]

This breaks Eglot f

Thanks for testing this patch, but as far as I know, this has already been fixed in the latest version in Emacs trunk.

[jcs090218]

Stupid? A one-liner? Could even be in Eglot upstream! And setting up a package doesn't need any config? And keeping it up to date? Are you sure you're not busy looking for an excuse to make a package, any package? Then why not make an useful one?

I'm gonna let you know I have seen your comment, but I will not reply since it will probably be a waste of my time. Cheers! :)