GNU bug report logs - #69605
[PATCH] Query Eglot version via package.el

Previous Next

To reply to this bug, email your comments to 69605 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: Philip Kaludercic <philipk <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Query Eglot version via package.el
Date: Thu, 07 Mar 2024 09:23:47 +0000

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

It seems to me that 'package-get-version' should already implement the
logic which is currently being reproduced when defining the constant
'eglot--version', so I suggest the following simplification:

[0001-Query-Eglot-version-via-package-el.patch (text/x-patch, attachment)]

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

-- 
	Philip Kaludercic on peregrine

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: 69605 <at> debbugs.gnu.org
Cc: João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#69605: [PATCH] Query Eglot version via package.el
Date: Fri, 03 May 2024 07:38:15 +0000

Ping?

Philip Kaludercic <philipk <at> posteo.net> writes:

> It seems to me that 'package-get-version' should already implement the
> logic which is currently being reproduced when defining the constant
> 'eglot--version', so I suggest the following simplification:
>
>>From 7566857ae869eb4cf01bde527eaedb8a3353f035 Mon Sep 17 00:00:00 2001
> From: Philip Kaludercic <philipk <at> posteo.net>
> Date: Thu, 7 Mar 2024 10:21:45 +0100
> Subject: [PATCH] Query Eglot version via package.el
>
> * lisp/progmodes/eglot.el (eglot--version): Remove constant.
> (eglot--connect): Use 'package-get-version'.
> ---
>  lisp/progmodes/eglot.el | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index f341428cac3..1ec186e75f8 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -545,13 +545,6 @@ 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.")
>  
>  (defconst eglot--symbol-kind-names
>    `((1 . "File") (2 . "Module")
> @@ -1585,8 +1578,10 @@ eglot--connect
>                                              'network))
>                                (emacs-pid))
>                              :clientInfo
> -                            `(:name "Eglot" ,@(when eglot--version
> -                                                `(:version ,eglot--version)))
> +                            (append
> +                             '(:name "Eglot")
> +                             (let ((v (package-get-version)))
> +                               (and v (list :version v))))
>                              ;; Maybe turn trampy `/ssh:foo <at> bar:/path/to/baz.py'
>                              ;; into `/path/to/baz.py', so LSP groks it.
>                              :rootPath (file-local-name
> -- 
> 2.44.0

-- 
	Philip Kaludercic on peregrine

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

From: João Távora <joaotavora <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 69605 <at> debbugs.gnu.org
Subject: Re: bug#69605: [PATCH] Query Eglot version via package.el
Date: Fri, 3 May 2024 12:54:05 +0100

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

On Fri, May 3, 2024 at 8:38 AM Philip Kaludercic <philipk <at> posteo.net> wrote:

> Ping?
>

Patch looks good to me, I'm generally in favour of things that replace
Eglot-rolled facilities with standard ones.

But I haven't tested it.  What will `package-get-version` say of a master
build of Emacs where lisp/progmodes/eglot.el has changes that haven't
been published to GNU Elpa yet?  Does it even matter?

Anyway, I think the change is safe to install, just please make sure it
doesn't trip the GitHub CI (it shouldn't).

João

[Message part 2 (text/html, inline)]

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: João Távora <joaotavora <at> gmail.com>
Cc: 69605 <at> debbugs.gnu.org
Subject: Re: bug#69605: [PATCH] Query Eglot version via package.el
Date: Fri, 03 May 2024 18:13:56 +0000

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

> On Fri, May 3, 2024 at 8:38 AM Philip Kaludercic <philipk <at> posteo.net> wrote:
>
>> Ping?
>>
>
> Patch looks good to me, I'm generally in favour of things that replace
> Eglot-rolled facilities with standard ones.
>
> But I haven't tested it.  What will `package-get-version` say of a master
> build of Emacs where lisp/progmodes/eglot.el has changes that haven't
> been published to GNU Elpa yet?  Does it even matter?

It will just extract the value of the version header of the compiled
file.

> Anyway, I think the change is safe to install, just please make sure it
> doesn't trip the GitHub CI (it shouldn't).

The three tested versions on GitHub all appear to pass the test.  I'll
push the changes to emacs.git for now.

> João

-- 
	Philip Kaludercic on peregrine

Previous Next

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