* bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
@ 2023-03-15 11:53 Felician Nemeth
2023-03-15 20:38 ` João Távora
[not found] ` <874jdipfp5.fsf@posteo.net>
0 siblings, 2 replies; 15+ messages in thread
From: Felician Nemeth @ 2023-03-15 11:53 UTC (permalink / raw)
To: 62198; +Cc: joaotavora
[-- Attachment #1: Type: text/plain, Size: 317 bytes --]
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.
[-- 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: 3436 bytes --]
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
\f
;;; 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")
(3 . "Namespace") (4 . "Package") (5 . "Class")
@@ -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"))
;; 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..f17b96848f 100644
--- a/test/lisp/progmodes/eglot-tests.el
+++ b/test/lisp/progmodes/eglot-tests.el
@@ -399,6 +399,24 @@ eglot-test-auto-reconnect
(while (process-live-p proc) (accept-process-output nil 0.5)))
(should (not (eglot-current-server)))))))
+(ert-deftest eglot-test-client-info ()
+ "Check clientInfo in Eglot's initialize request."
+ (skip-unless (executable-find "pylsp"))
+ (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"))))))))))
+
(ert-deftest eglot-test-rust-analyzer-watches-files ()
"Start rust-analyzer. Notify it when a critical file changes."
(skip-unless (executable-find "rust-analyzer"))
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
2023-03-15 11:53 bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request Felician Nemeth
@ 2023-03-15 20:38 ` João Távora
2023-03-16 16:47 ` Felician Nemeth
[not found] ` <874jdipfp5.fsf@posteo.net>
1 sibling, 1 reply; 15+ messages in thread
From: João Távora @ 2023-03-15 20:38 UTC (permalink / raw)
To: Felician Nemeth; +Cc: 62198
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
> \f
> ;;; 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
2023-03-15 20:38 ` João Távora
@ 2023-03-16 16:47 ` Felician Nemeth
2023-03-17 8:25 ` Michael Albinus
0 siblings, 1 reply; 15+ messages in thread
From: Felician Nemeth @ 2023-03-16 16:47 UTC (permalink / raw)
To: João Távora; +Cc: 62198
[-- 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
2023-03-16 16:47 ` Felician Nemeth
@ 2023-03-17 8:25 ` Michael Albinus
2023-03-19 12:15 ` Felician Nemeth
0 siblings, 1 reply; 15+ messages in thread
From: Michael Albinus @ 2023-03-17 8:25 UTC (permalink / raw)
To: Felician Nemeth; +Cc: 62198, João Távora
Felician Nemeth <felician.nemeth@gmail.com> writes:
Hi Felician,
> 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.
FTR, trampver.el is always generated from trampver.el.in in the Tramp
repository, see <https://git.savannah.gnu.org/cgit/tramp.git/plain/lisp/trampver.el.in>.
Best regards, Michael.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
2023-03-17 8:25 ` Michael Albinus
@ 2023-03-19 12:15 ` Felician Nemeth
2023-03-19 13:23 ` João Távora
0 siblings, 1 reply; 15+ messages in thread
From: Felician Nemeth @ 2023-03-19 12:15 UTC (permalink / raw)
To: Michael Albinus; +Cc: 62198, João Távora
Hi Michael,
> FTR, trampver.el is always generated from trampver.el.in in the Tramp
> repository, see <https://git.savannah.gnu.org/cgit/tramp.git/plain/lisp/trampver.el.in>.
Ah, okay, sorry.
At any rate, I think a Makefile would be an overkill for this little
feature. So maybe clientInfo should just send the name of the client if
João is not happy with the previous approach finding out the version of
Eglot.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
2023-03-19 12:15 ` Felician Nemeth
@ 2023-03-19 13:23 ` João Távora
2023-03-22 16:05 ` Felician Nemeth
0 siblings, 1 reply; 15+ messages in thread
From: João Távora @ 2023-03-19 13:23 UTC (permalink / raw)
To: Felician Nemeth; +Cc: 62198, Michael Albinus
On Sun, Mar 19, 2023 at 12:15 PM Felician Nemeth
<felician.nemeth@gmail.com> wrote:
>
> Hi Michael,
>
> > FTR, trampver.el is always generated from trampver.el.in in the Tramp
> > repository, see <https://git.savannah.gnu.org/cgit/tramp.git/plain/lisp/trampver.el.in>.
>
> Ah, okay, sorry.
>
> At any rate, I think a Makefile would be an overkill for this little
> feature.
FWIW, my other SLY extension also has a complicated "scan own file"
thing. Known to not work extremely well/be very useful...
> So maybe clientInfo should just send the name of the client if
> João is not happy with the previous approach finding out the version of
> Eglot.
I don't know if I'm happy with it, since I'm not familiar with what
it does.
The version could be useful. The problem is that it is tricky
to get right, meaning something that you can really rely on. Ideally
I'd like to know two things in the log transcript some hypothetical
bug reporter shares:
1. Is this an unmodified GNU(-devel) ELPA version of Eglot installed by
the only officially supported `M-x package-install` method? If so,
what version exactly?
2. This is something else, a user compilation, a straight-installed
package, nix installation (never tried these, but they show
up in the wild a lot, and are problematic like in
https://github.com/joaotavora/eglot/discussions/1182).
I don't think any more granularity within 2 would be very useful,
especially to the server devs. Although it _could_ be useful to
somehow debug -- not via LSP messages to server, but in the events
log as an internal message -- the versions of the packages that
Eglot depends on. But that's a broader idea, I think, and one
more suitable for a future M-x report-bug-in-elpa-package.
If this is not easy to do without complex libraries, i'd prefer just to
send the client name via LSP.
João
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
2023-03-19 13:23 ` João Távora
@ 2023-03-22 16:05 ` Felician Nemeth
2023-03-22 18:40 ` João Távora
0 siblings, 1 reply; 15+ messages in thread
From: Felician Nemeth @ 2023-03-22 16:05 UTC (permalink / raw)
To: João Távora; +Cc: 62198, Michael Albinus
[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]
>> So maybe clientInfo should just send the name of the client if
>> João is not happy with the previous approach finding out the version of
>> Eglot.
>
> I don't know if I'm happy with it, since I'm not familiar with what
> it does.
This is eglot--version in my original patch:
(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.")
It does something similar to sly-version, but instead of manually
parsing the source file it relies on `lm-version' of the lisp-mnt
package. (package.el also uses lisp-mnt to parse "header comments".)
However, lm-version needs a file argument to parse. If eglot.el is
byte-compiled, then byte-compile-current-file will be set to eglot.el
during compile-time and eglot--version won't have a run-time calculation
cost.
> The version could be useful. The problem is that it is tricky
> to get right, meaning something that you can really rely on. Ideally
> I'd like to know two things in the log transcript some hypothetical
> bug reporter shares:
>
> 1. Is this an unmodified GNU(-devel) ELPA version of Eglot installed by
> the only officially supported `M-x package-install` method? If so,
> what version exactly?
>
> 2. This is something else, a user compilation, a straight-installed
> package, nix installation (never tried these, but they show
> up in the wild a lot, and are problematic like in
> https://github.com/joaotavora/eglot/discussions/1182).
>
> I don't think any more granularity within 2 would be very useful,
> especially to the server devs.
I don't know how to answer these questions.
> Although it _could_ be useful to somehow debug -- not via LSP messages
> to server, but in the events log as an internal message -- the
> versions of the packages that Eglot depends on. But that's a broader
> idea, I think, and one more suitable for a future M-x
> report-bug-in-elpa-package.
I think it is possible write the versions of Eglot and its dependencies
into eglot-events-buffer with the help of list-mnt, but I don't think
Eglot should send this in clientInfo.
> If this is not easy to do without complex libraries, i'd prefer just to
> send the client name via LSP.
I've attached a simple patch that sends just the client name if you
decide to go this way.
Thanks.
[-- 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: 1206 bytes --]
From d88fbc30335fa4f68e1dfa339fb9e23f80eadbef 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--connect): Send clientInfo.
---
lisp/progmodes/eglot.el | 1 +
1 file changed, 1 insertion(+)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 037cc87148..91fa00d4b7 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1310,6 +1310,7 @@ eglot--connect
(eq (jsonrpc-process-type server)
'network))
(emacs-pid))
+ :clientInfo '(:name "Eglot")
;; Maybe turn trampy `/ssh:foo@bar:/path/to/baz.py'
;; into `/path/to/baz.py', so LSP groks it.
:rootPath (file-local-name
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
2023-03-22 16:05 ` Felician Nemeth
@ 2023-03-22 18:40 ` João Távora
2023-03-23 16:03 ` Felician Nemeth
0 siblings, 1 reply; 15+ messages in thread
From: João Távora @ 2023-03-22 18:40 UTC (permalink / raw)
To: Felician Nemeth; +Cc: 62198, Michael Albinus
Felician Nemeth <felician.nemeth@gmail.com> writes:
>> I don't think any more granularity within 2 would be very useful,
>> especially to the server devs.
>
> I don't know how to answer these questions.
I'm not asking you :-) These are questions I ask myself when facing bug
reports. And usually I don't have a good way of answering except for
drilling the bug reporter.
I agree this most of this information shouldn't probably be sent over to
the server.
>> Although it _could_ be useful to somehow debug -- not via LSP messages
>> to server, but in the events log as an internal message -- the
>> versions of the packages that Eglot depends on. But that's a broader
>> idea, I think, and one more suitable for a future M-x
>> report-bug-in-elpa-package.
>
> I think it is possible write the versions of Eglot and its dependencies
> into eglot-events-buffer with the help of list-mnt, but I don't think
> Eglot should send this in clientInfo.
Yes, of course. It's debugging information. Currently the manual
advises users to list the files in 'package-user-dir', which is a really
poor way of telling the versions of ELPA packages, but it's the best
I've come up with. What I meant is I'd like to have a way to produce a
listing of packages (required and optional, like
markdown/company/yasnippet) for inclusion in bug reports.
>> If this is not easy to do without complex libraries, i'd prefer just to
>> send the client name via LSP.
>
> I've attached a simple patch that sends just the client name if you
> decide to go this way.
Thanks, I've pushed it to master.
João
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request
2023-03-22 18:40 ` João Távora
@ 2023-03-23 16:03 ` Felician Nemeth
0 siblings, 0 replies; 15+ messages in thread
From: Felician Nemeth @ 2023-03-23 16:03 UTC (permalink / raw)
To: João Távora; +Cc: 62198
> Currently the manual advises users to list the files in
> 'package-user-dir', which is a really poor way of telling the versions
> of ELPA packages, but it's the best I've come up with. What I meant
> is I'd like to have a way to produce a listing of packages (required
> and optional, like markdown/company/yasnippet) for inclusion in bug
> reports.
The code below might be a good starting point for this. It is inspired
by `package-compute-transaction'. It seems to work even with Debian's
package manager ("apt install elpa-project"). So I'd like to think that
a potential failure in the code is a sign that the bug reporter has a
non-standard installation.
(require 'find-func)
(require 'package)
(defun my-list-dependencies (packages &optional seen versions)
"Retrun the versions of dependencies of PACKAGES.
Argument PACKAGES is a list of symbols. SEEN, VERSION are used
internally."
(while packages
(let ((package (pop packages)))
(unless (memq package seen)
(push package seen)
(let* ((file (condition-case error
(find-library-name (symbol-name package))
(error nil)))
(pkg-desc (if file
(with-temp-buffer
(insert-file-contents file)
(package-buffer-info))
(if (eq package 'emacs)
(package-desc-create :name "emacs"
:version (version-to-list
emacs-version))
(package-desc-create :name (symbol-name package)
:version '(0))))))
(push (package-desc-full-name pkg-desc) versions)
(setq packages (append packages
(mapcar #'car
(package-desc-reqs pkg-desc))))))))
versions)
(my-list-dependencies '(eglot markdown company other-optional-dependency))
^ permalink raw reply [flat|nested] 15+ messages in thread
* (byte-compile '(append '(1 2) '(3 4)))
[not found] ` <87bk7p57yz.fsf@posteo.net>
@ 2024-03-16 12:16 ` Felician Nemeth
2024-03-16 12:46 ` Philip Kaludercic
0 siblings, 1 reply; 15+ messages in thread
From: Felician Nemeth @ 2024-03-16 12:16 UTC (permalink / raw)
To: emacs-devel; +Cc: Philip Kaludercic
(disassemble (byte-compile '(append '(1 2) '(3 4))))
resuts in
byte code:
args: nil
0 constant append
1 constant (1 2)
2 constant (3 4)
3 call 2
4 return
Instead I expected it to be something like
byte code:
args: nil
0 constant 1
1 constant 2
2 constant 3
3 constant 4
4 list4
5 return
I've never looked at byte-code optimization before, and I'm guessing
this is not a huge improvement, but I still wonder when all the
arguments of side-effect-free function are constants would it make sense
to calculate the result at compile time.
Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: (byte-compile '(append '(1 2) '(3 4)))
2024-03-16 12:16 ` (byte-compile '(append '(1 2) '(3 4))) Felician Nemeth
@ 2024-03-16 12:46 ` Philip Kaludercic
2024-03-16 13:23 ` Basil L. Contovounesios
0 siblings, 1 reply; 15+ messages in thread
From: Philip Kaludercic @ 2024-03-16 12:46 UTC (permalink / raw)
To: Felician Nemeth; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]
Felician Nemeth <felician.nemeth@gmail.com> writes:
> (disassemble (byte-compile '(append '(1 2) '(3 4))))
>
> resuts in
>
> byte code:
> args: nil
> 0 constant append
> 1 constant (1 2)
> 2 constant (3 4)
> 3 call 2
> 4 return
>
> Instead I expected it to be something like
>
> byte code:
> args: nil
> 0 constant 1
> 1 constant 2
> 2 constant 3
> 3 constant 4
> 4 list4
> 5 return
>
> I've never looked at byte-code optimization before, and I'm guessing
> this is not a huge improvement, but I still wonder when all the
> arguments of side-effect-free function are constants would it make sense
> to calculate the result at compile time.
`byte-optimize-append' mentions:
;; There is (probably) too much code relying on `append' to return a
;; new list for us to do full constant-folding; these transformations
;; preserve the allocation semantics.
I am not sure what the danger is in the case of constant, quoted lists,
but I am not familiar with the byte compiler either. It seems like this
[-- Attachment #2: Type: text/plain, Size: 650 bytes --]
diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index f75be3f71ad..e6f18590705 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -1599,6 +1599,12 @@ byte-optimize-append
(cdr args))
(cdr newargs)))
+ ;; (append '(C1...) ... '(C2...)) -> (append C1... ... C2...)
+ ((cl-loop for arg in args
+ always (and (eq (car arg) 'quote)
+ (proper-list-p (cdr arg))))
+ `',(mapcan #'cadr args))
+
;; non-terminal arg
((cdr args)
(cond
[-- Attachment #3: Type: text/plain, Size: 151 bytes --]
would do the trick, and the byte-code is even better:
byte code:
args: nil
0 constant (1 2 3 4)
1 return
--
Philip Kaludercic on peregrine
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: (byte-compile '(append '(1 2) '(3 4)))
2024-03-16 12:46 ` Philip Kaludercic
@ 2024-03-16 13:23 ` Basil L. Contovounesios
2024-03-16 13:45 ` Felician Nemeth
2024-03-16 13:55 ` Philip Kaludercic
0 siblings, 2 replies; 15+ messages in thread
From: Basil L. Contovounesios @ 2024-03-16 13:23 UTC (permalink / raw)
To: Philip Kaludercic; +Cc: Felician Nemeth, emacs-devel
Philip Kaludercic [2024-03-16 12:46 +0000] wrote:
> I am not sure what the danger is in the case of constant, quoted lists,
> but I am not familiar with the byte compiler either. It seems like this
>
> diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
> index f75be3f71ad..e6f18590705 100644
> --- a/lisp/emacs-lisp/byte-opt.el
> +++ b/lisp/emacs-lisp/byte-opt.el
> @@ -1599,6 +1599,12 @@ byte-optimize-append
> (cdr args))
> (cdr newargs)))
>
> + ;; (append '(C1...) ... '(C2...)) -> (append C1... ... C2...)
> + ((cl-loop for arg in args
> + always (and (eq (car arg) 'quote)
> + (proper-list-p (cdr arg))))
> + `',(mapcan #'cadr args))
> +
> ;; non-terminal arg
> ((cdr args)
> (cond
>
>
> would do the trick, and the byte-code is even better:
>
> byte code:
> args: nil
> 0 constant (1 2 3 4)
> 1 return
Is this correct? According to its docstring, append's last argument
must be eq to the tail of its return value.
--
Basil
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: (byte-compile '(append '(1 2) '(3 4)))
2024-03-16 13:23 ` Basil L. Contovounesios
@ 2024-03-16 13:45 ` Felician Nemeth
2024-03-16 13:55 ` Philip Kaludercic
1 sibling, 0 replies; 15+ messages in thread
From: Felician Nemeth @ 2024-03-16 13:45 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: Philip Kaludercic, emacs-devel
> According to its docstring, append's last argument must be eq to the
> tail of its return value.
So that is why the compiler does optimize out (append '(1 2) '(3 4)) in
(disassemble (byte-compile '(append (append '(1 2) '(3 4)) '(5 6)))) , but
does not optimize the same expression out in
(disassemble (byte-compile '(append (append '(1 2) '(3 4))))) ?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: (byte-compile '(append '(1 2) '(3 4)))
2024-03-16 13:23 ` Basil L. Contovounesios
2024-03-16 13:45 ` Felician Nemeth
@ 2024-03-16 13:55 ` Philip Kaludercic
2024-03-16 23:42 ` Basil L. Contovounesios
1 sibling, 1 reply; 15+ messages in thread
From: Philip Kaludercic @ 2024-03-16 13:55 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: Felician Nemeth, emacs-devel
"Basil L. Contovounesios" <basil@contovou.net> writes:
> Philip Kaludercic [2024-03-16 12:46 +0000] wrote:
>
>> I am not sure what the danger is in the case of constant, quoted lists,
>> but I am not familiar with the byte compiler either. It seems like this
>>
>> diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
>> index f75be3f71ad..e6f18590705 100644
>> --- a/lisp/emacs-lisp/byte-opt.el
>> +++ b/lisp/emacs-lisp/byte-opt.el
>> @@ -1599,6 +1599,12 @@ byte-optimize-append
>> (cdr args))
>> (cdr newargs)))
>>
>> + ;; (append '(C1...) ... '(C2...)) -> (append C1... ... C2...)
>> + ((cl-loop for arg in args
>> + always (and (eq (car arg) 'quote)
>> + (proper-list-p (cdr arg))))
>> + `',(mapcan #'cadr args))
>> +
>> ;; non-terminal arg
>> ((cdr args)
>> (cond
>>
>>
>> would do the trick, and the byte-code is even better:
>>
>> byte code:
>> args: nil
>> 0 constant (1 2 3 4)
>> 1 return
>
> Is this correct? According to its docstring, append's last argument
> must be eq to the tail of its return value.
This little test indicates that it would still be eq:
((macro . (lambda (arg)
`(eq (cddadr (byte-optimize-append '(append '(1 2) ,arg)))
,arg)))
'(3 4)) ;=> t
since the mapcan or rather nconc makes the same promise.
--
Philip Kaludercic on peregrine
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: (byte-compile '(append '(1 2) '(3 4)))
2024-03-16 13:55 ` Philip Kaludercic
@ 2024-03-16 23:42 ` Basil L. Contovounesios
0 siblings, 0 replies; 15+ messages in thread
From: Basil L. Contovounesios @ 2024-03-16 23:42 UTC (permalink / raw)
To: Philip Kaludercic; +Cc: Felician Nemeth, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]
Philip Kaludercic [2024-03-16 13:55 +0000] wrote:
> "Basil L. Contovounesios" <basil@contovou.net> writes:
>> Philip Kaludercic [2024-03-16 12:46 +0000] wrote:
>>
>>> I am not sure what the danger is in the case of constant, quoted lists,
>>> but I am not familiar with the byte compiler either. It seems like this
>>>
>>> --- a/lisp/emacs-lisp/byte-opt.el
>>> +++ b/lisp/emacs-lisp/byte-opt.el
>>> @@ -1599,6 +1599,12 @@ byte-optimize-append
>>> (cdr args))
>>> (cdr newargs)))
>>>
>>> + ;; (append '(C1...) ... '(C2...)) -> (append C1... ... C2...)
>>> + ((cl-loop for arg in args
>>> + always (and (eq (car arg) 'quote)
>>> + (proper-list-p (cdr arg))))
>>> + `',(mapcan #'cadr args))
>>> +
>>> ;; non-terminal arg
>>> ((cdr args)
>>> (cond
>>>
>>> would do the trick, and the byte-code is even better:
>>>
>>> byte code:
>>> args: nil
>>> 0 constant (1 2 3 4)
>>> 1 return
>>
>> Is this correct? According to its docstring, append's last argument
>> must be eq to the tail of its return value.
>
> This little test indicates that it would still be eq:
>
> ((macro . (lambda (arg)
> `(eq (cddadr (byte-optimize-append '(append '(1 2) ,arg)))
> ,arg)))
> '(3 4)) ;=> t
>
> since the mapcan or rather nconc makes the same promise.
Right, it seems straightforward to keep the resulting tail eq to the
last argument. I'm too squeamish to destructively modify the input
form, and Emacs doesn't build with the diff above, so I'm guessing you
meant something like this instead:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: append.diff --]
[-- Type: text/x-diff, Size: 857 bytes --]
diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index f75be3f71ad..f0e543319c3 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -1630,6 +1630,13 @@ byte-optimize-append
;; (append X) -> X
((null newargs) arg)
+ ;; (append '(X...) ... '(Y...)) -> '(X... Y...)
+ ((and (eq (car-safe arg) 'quote)
+ (cl-loop for newarg in newargs
+ always (and (eq (car-safe newarg) 'quote)
+ (proper-list-p (cadr newarg)))))
+ `',(apply #'append (mapcar #'cadr (nreverse (cons arg newargs)))))
+
;; (append ... (list Xs...) nil) -> (append ... (list Xs...))
((and (null arg) (eq (car-safe prev) 'list))
(cons (car form) (nreverse newargs)))
[-- Attachment #3: Type: text/plain, Size: 156 bytes --]
But then the problem is that you can't e.g.
(setcar (append '(0) '(1)) t)
without it looking like the program is modifying a constant list.
--
Basil
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-03-16 23:42 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-15 11:53 bug#62198: [PATCH] Eglot: Send clientInfo during the initialize request Felician Nemeth
2023-03-15 20:38 ` João Távora
2023-03-16 16:47 ` Felician Nemeth
2023-03-17 8:25 ` Michael Albinus
2023-03-19 12:15 ` Felician Nemeth
2023-03-19 13:23 ` João Távora
2023-03-22 16:05 ` Felician Nemeth
2023-03-22 18:40 ` João Távora
2023-03-23 16:03 ` Felician Nemeth
[not found] ` <874jdipfp5.fsf@posteo.net>
[not found] ` <87cys6t734.fsf@betli.tmit.bme.hu>
[not found] ` <87r0gmnjq4.fsf@posteo.net>
[not found] ` <87o7bprr04.fsf@betli.tmit.bme.hu>
[not found] ` <87bk7p57yz.fsf@posteo.net>
2024-03-16 12:16 ` (byte-compile '(append '(1 2) '(3 4))) Felician Nemeth
2024-03-16 12:46 ` Philip Kaludercic
2024-03-16 13:23 ` Basil L. Contovounesios
2024-03-16 13:45 ` Felician Nemeth
2024-03-16 13:55 ` Philip Kaludercic
2024-03-16 23:42 ` Basil L. Contovounesios
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.