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

[João Távora]

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

> I originally posted this patch on github:
> https://github.com/joaotavora/eglot/pull/818
>
> João requested to resubmit it here.
>
> To recap: 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.
>
> From 5fe9d054845382ce74c29bb338ffc737b0770542 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-client-info): New test.
> ---
>  lisp/progmodes/eglot.el            | 12 ++++++++++++
>  test/lisp/progmodes/eglot-tests.el | 18 ++++++++++++++++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index 037cc87148..f89c47ac4f 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -410,6 +410,14 @@ eglot-withhold-process-id
>
>  ;;; Constants
>  ;;;
> +(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?  Why is it nil if Eglot is not
byte-compiled, couldn't we get it by looking at load-file-name?

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

> @@ -1310,6 +1318,10 @@ eglot--connect
>                                          (eq (jsonrpc-process-type server)
>                                              'network))
>                                (emacs-pid))
> +                            :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.

> +  (eglot--with-fixture
> +   `(("project" . (("coiso.py" . "def coiso: pass"))))
> +   (with-current-buffer
> +       (eglot--find-file-noselect "project/coiso.py")
> +     (eglot--sniffing (:client-requests c-reqs)
> +       (should (eglot--tests-connect 10))
> +       (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" (or version "0"))))))))))

Thanks.  Do we really need a test for this?  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.

João