bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Felician Nemeth
Subject: bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
Date: Thu, 16 Mar 2023 17:47:03 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

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

Attachment: 0001-Eglot-Send-clientInfo-during-the-initialize-request.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]