GNU bug report logs - #76683
[PATCH] Eglot: Implement additionalPropertiesSupport for showMessage

Previous Next

Full log

Message #14 received at 76683 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Felician Nemeth <felician.nemeth <at> gmail.com>
Cc: 76683 <at> debbugs.gnu.org
Subject: Re: bug#76683: [PATCH] Eglot: Implement additionalPropertiesSupport
 for showMessage
Date: Sun, 02 Mar 2025 22:24:49 +0000

Felician Nemeth <felician.nemeth <at> gmail.com> writes:

>> * Why did you remove the gh#627 reference?  Not relevant anymore?
>
> I think the note about gh#627 warned the reader that a "vector to list
> conversion" was necessary.  However, mapcar can work with vectors, so
> the reference is not relevant anymore.

Can't see that reference in github#627, but can't see the relevance
either, so OK to erase.

>> * Could eglot--lambda be used for simplicity (I know it wasn't used
>> before)
>
> I think it won't be simpler, but it might be easier to understand the
> intent of the code.  However, I don't know how it is possible to rewrite
> the lambda.  How can we access the original object from the body of
> eglot--lambda?

Ah. You can't, indeed.  eglot--lambda doesn't have some like &whole arg
support (because cl-destructuring-bind also doesn't, unfortunatley).  I
have a patch for that, but could only find one place to use it so I held
it back.  But it seems there are at least two, so I'll attach the patch
after my sig and you tell me if you think it's worth it.  Possibly it's
not, at least not yet.  Unless you have things in eglot-x.el that could
take advantage.

> The patch has this: (lambda (a) (cons (plist-get a :title) a))
>
> It should be turned into something like this:
>
> (eglot--lambda ((MessageActionItem) title)
>    (cons title orignal-object))
>
>> * Are you 100% we'll return an 'equal' object for servers that don't
>> make use of this feature?
>
> The specification says: "result: the selected MessageActionItem".  If
> the server does not send additional properties, the new version will
> return what the original one returns: an object with a single title
> property.  So this patch should not cause any trouble to servers
> following the specification.

Makes sense.

> Although, there is a corner case when the new version gives different
> result.  The original version can return (:title "OK") for an empty
> action list, which is not in line with the specification.  The updated
> patch returns :null in this case.

Alright.

>> * In the original version it was obvious the structure returned, now
>> not so much.   Maybe
>> a very short comment showing what the typical object looks like.
>
> I added a comment and extended the docstring.
>
>> After evaluating this, and doing changes (if any), feel free to push.
>
> After your last email, I requested inclusion for the emacs group at
> savannah three weeks ago.  Nothing happened since then, which I
> interpret as polite rejection.

Hmm, not sure about the rejection (or the polite, tbh).  I'd ping them
again.

I pushed a very slightly simplified version of the patch.  Check if it
is OK (I didn't test).  We can fixup later if I introduced a bug.

João

Here's the patch I was talkng about:

commit f6df4e9154f6ba1b4c9358d7da4470fdd8fc73d8
Author: João Távora <joaotavora <at> gmail.com>
Date:   Wed Feb 5 12:14:43 2025 +0000

    Eglot: add &whole support to eglot--lambda
    
    * lisp/progmodes/eglot.el (eglot--lambda): Add &whole support.
    (eglot--workspace-symbols): Use it.

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 7b1c174c4d7..17ad47ab64a 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -870,8 +870,17 @@ eglot--lambda
   "Function of args CL-LAMBDA-LIST for processing INTERFACE objects.
 Honor `eglot-strict-mode'."
   (declare (indent 1) (debug (sexp &rest form)))
-  (let ((e (cl-gensym "jsonrpc-lambda-elem")))
-    `(lambda (,e) (cl-block nil (eglot--dbind ,cl-lambda-list ,e ,@body)))))
+  (let ((e (cl-gensym "jsonrpc-lambda-elem"))
+        whole)
+    (when-let* ((whole-pos (cl-position '&whole cl-lambda-list)))
+      (setq whole (nth (1+ whole-pos) cl-lambda-list)
+            cl-lambda-list (cl-subseq cl-lambda-list 0 whole-pos)))
+    `(lambda (,e)
+       ,(if whole
+            `(cl-block nil
+               (let ((,whole ,e))
+                 (eglot--dbind ,cl-lambda-list ,e ,@body)))
+            `(cl-block nil (eglot--dbind ,cl-lambda-list ,e ,@body))))))
 
 (cl-defmacro eglot--dcase (obj &rest clauses)
   "Like `pcase', but for the LSP object OBJ.
@@ -3107,16 +3116,15 @@ eglot--workspace-symbols
   (with-current-buffer (or buffer (current-buffer))
     (eglot-server-capable-or-lose :workspaceSymbolProvider)
     (mapcar
-     (lambda (wss)
-       (eglot--dbind ((WorkspaceSymbol) name containerName kind) wss
-         (propertize
-          (format "%s%s %s"
-                  (if (zerop (length containerName)) ""
-                    (concat (propertize containerName 'face 'shadow) " "))
-                  name
-                  (propertize (alist-get kind eglot--symbol-kind-names "Unknown")
-                              'face 'shadow))
-          'eglot--lsp-workspaceSymbol wss)))
+     (eglot--lambda ((WorkspaceSymbol) name containerName kind &whole wss)
+       (propertize
+        (format "%s%s %s"
+                (if (zerop (length containerName)) ""
+                  (concat (propertize containerName 'face 'shadow) " "))
+                name
+                (propertize (alist-get kind eglot--symbol-kind-names "Unknown")
+                            'face 'shadow))
+        'eglot--lsp-workspaceSymbol wss))
      (eglot--request (eglot--current-server-or-lose) :workspace/symbol
                      `(:query ,pat)))))
 

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.