GNU bug report logs - #62198
[PATCH] Eglot: Send clientInfo during the initialize request

Previous Next

Full log

View this message in rfc822 format

From: Felician Nemeth <felician.nemeth <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 62198 <at> debbugs.gnu.org
Subject: bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
Date: Thu, 16 Mar 2023 17:47:03 +0100

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

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

[0001-Eglot-Send-clientInfo-during-the-initialize-request.patch (text/x-diff, attachment)]

Previous Next

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