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

Previous Next

To reply to this bug, email your comments to 76683 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

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

From: Felician Nemeth <felician.nemeth <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Eglot: Implement additionalPropertiesSupport for showMessage
Date: Sun, 02 Mar 2025 14:39:18 +0100

[Message part 1 (text/plain, inline)]

Hi João,

This small patch implements a harmless feature introduced in LSP version
3.16:
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification/#window_showMessageRequest

It allows me to implement a rust-analyzer feautre in a simple, stateless
manner.

Thank you,
Felicián

[0001-Eglot-Implement-additionalPropertiesSupport-for-show.patch (text/x-diff, attachment)]

Message #8 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, 2 Mar 2025 13:59:44 +0000

Looks good.  Minor comments.

* Why did you remove the gh#627 reference?  Not relevant anymore?
* Could eglot--lambda be used for simplicity (I know it wasn't used before)
* Are you 100% we'll return an 'equal' object for servers that don't
make use of this feature?
* 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.

After evaluating this, and doing changes (if any), feel free to push.

João

On Sun, Mar 2, 2025 at 1:40 PM Felician Nemeth
<felician.nemeth <at> gmail.com> wrote:
>
> Hi João,
>
> This small patch implements a harmless feature introduced in LSP version
> 3.16:
> https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification/#window_showMessageRequest
>
> It allows me to implement a rust-analyzer feautre in a simple, stateless
> manner.
>
> Thank you,
> Felicián
>


-- 
João Távora

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

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

[Message part 1 (text/plain, inline)]

> * 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.

> * 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?

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.

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.

> * 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.

I've attached the updated patch.

Thanks again,
Felicián

[0001-Eglot-Implement-additionalPropertiesSupport-for-show.patch (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

> João
>
> On Sun, Mar 2, 2025 at 1:40 PM Felician Nemeth
> <felician.nemeth <at> gmail.com> wrote:
>>
>> Hi João,
>>
>> This small patch implements a harmless feature introduced in LSP version
>> 3.16:
>> https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification/#window_showMessageRequest
>>
>> It allows me to implement a rust-analyzer feautre in a simple, stateless
>> manner.
>>
>> Thank you,
>> Felicián
>>

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)))))
 

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

From: Felician Nemeth <felician.nemeth <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 76683 <at> debbugs.gnu.org
Subject: Re: bug#76683: [PATCH] Eglot: Implement additionalPropertiesSupport
 for showMessage
Date: Sat, 08 Mar 2025 17:40:43 +0100

João Távora <joaotavora <at> gmail.com> writes:

> 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.

I don't see use cases for it at the moment, but I keep this in mind and
let you know when something comes up.

> 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.

The pushed version works for me.  Thank you,

Felicián

Previous Next

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