bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request

[Felician Nemeth <felician.nemeth@gmail.com>]

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

>> +(defconst eglot--version
>> +  (eval-when-compile
>> +    (when byte-compile-current-file
>> +      (require 'lisp-mnt)
>> +      (lm-version byte-compile-current-file)))
>> +  "The version as a string of this version of Eglot.
>> +It is nil if Eglot is not byte-complied.")
>
> I'm not familiar with this lisp-mnt.el library.  Is it the kosher way to
> get version introspection for Elisp libs?

I haven't found the prior art for this.  trampver.el repeats the header
info, so when the version changes it should be updated two places, which
(I think) is an antipattern.  For "normal" users, Eglot should come with
Emacs or be installed from ELPA.  In both cases, Eglot is byte-compiled.

> Why is it nil if Eglot is not byte-compiled, couldn't we get it by
> looking at load-file-name?

Yes, that's a possibily, but that won't be perfect either.  I tend to
eval-buffer when I work on Eglot.

> Can we somehow get the Emacs.git SHA in there as well?

There is `emacs-repository-get-version', but according to its docstring
it doesn't work all the time.  trampver.el is a good example how
complicated its usage.

Do you intend to rely on the clientInfo in bug reports?  I think it's
safer to ask for the exact details if the user is running a not released
version.

>> +                            :clientInfo (if eglot--version
>> +                                            `(:name "Eglot"
>> +                                              :version ,eglot--version)
>> +                                          '(:name "Eglot"))
>
> I'd rather just :name "Eglot" :version "unknown" if we don't have it.

"Version" is optional.  I think it shouldn't be specified if it is
unknown.  Nevertheless, I updated the patch to send "unknown" in this
case.

> Do we really need a test for this?

I wrote the test after Stefan's implicit request.  I'm OK with not having
a test.

> I suppose it's good practice, but
> seems a but too much.  We could put this check in some other "basic"
> test, save a bit of time.

I've updated the patch.  It now extends another test.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Eglot-Send-clientInfo-during-the-initialize-request.patch --]
[-- Type: text/x-diff, Size: 3600 bytes --]

From ed48b73457048f89bb5132eefc2c3d2fa94e68f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Felici=C3=A1n=20N=C3=A9meth?= <felician.nemeth@gmail.com>
Date: Wed, 15 Mar 2023 12:34:06 +0100
Subject: [PATCH] Eglot: Send clientInfo during the initialize request

clientInfo arrived in LSP 3.15.0.  LSP clients can use clientInfo to
identify themselves in the initialize request.  This is generally
useful for various debugging tasks.

* lisp/progmodes/eglot.el (eglot--version): New defconst.
(eglot--connect): Send clientInfo using eglot--version.
* test/lisp/progmodes/eglot-tests.el
(eglot-test-basic-diagnostics): Test clientInfo as well.
---
 lisp/progmodes/eglot.el            | 10 ++++++++++
 test/lisp/progmodes/eglot-tests.el | 18 +++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 037cc87148..8ca0db85a7 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -410,6 +410,15 @@ eglot-withhold-process-id
 \f
 ;;; Constants
 ;;;
+(defconst eglot--version
+  (or (eval-when-compile
+        (when byte-compile-current-file
+          (require 'lisp-mnt)
+          (lm-version byte-compile-current-file)))
+      "unknown"))
+  "The version as a string of this version of Eglot.
+It is \"unknown\" if Eglot is not byte-complied.")
+
 (defconst eglot--symbol-kind-names
   `((1 . "File") (2 . "Module")
     (3 . "Namespace") (4 . "Package") (5 . "Class")
@@ -1310,6 +1319,7 @@ eglot--connect
                                         (eq (jsonrpc-process-type server)
                                             'network))
                               (emacs-pid))
+                            :clientInfo `(:name "Eglot" :version ,eglot--version)
                             ;; Maybe turn trampy `/ssh:foo@bar:/path/to/baz.py'
                             ;; into `/path/to/baz.py', so LSP groks it.
                             :rootPath (file-local-name
diff --git a/test/lisp/progmodes/eglot-tests.el b/test/lisp/progmodes/eglot-tests.el
index b95e527c51..a2ca35bd7d 100644
--- a/test/lisp/progmodes/eglot-tests.el
+++ b/test/lisp/progmodes/eglot-tests.el
@@ -435,15 +435,27 @@ eglot-test-rust-analyzer-watches-files
                         (= type 3))))))))))
 
 (ert-deftest eglot-test-basic-diagnostics ()
-  "Test basic diagnostics."
+  "Test basic diagnostics and clientInfo."
   (skip-unless (executable-find "clangd"))
   (eglot--with-fixture
       `(("diag-project" .
          (("main.c" . "int main(){froat a = 42.2; return 0;}"))))
     (with-current-buffer
         (eglot--find-file-noselect "diag-project/main.c")
-      (eglot--sniffing (:server-notifications s-notifs)
-        (eglot--tests-connect)
+      (eglot--sniffing (:server-notifications s-notifs
+                        :client-requests c-reqs)
+                       (eglot--tests-connect)
+        ;; Test clientInfo sent by Eglot.
+        ;; This simple test included here to save some time.
+        (eglot--wait-for (c-reqs 10)
+           (&key _id method params &allow-other-keys)
+         (when (string= method "initialize")
+           (let* ((clientInfo (plist-get params :clientInfo))
+                  (name (plist-get clientInfo :name))
+                  (version (plist-get clientInfo :version)))
+             (should (equal name "Eglot"))
+             (should (version<= "1.8" version)))))
+        ;; Test basic diagnostics.
         (eglot--wait-for (s-notifs 10)
             (&key _id method &allow-other-keys)
           (string= method "textDocument/publishDiagnostics"))
-- 
2.30.2