GNU bug report logs - #64543
[PATCH] package-report-bug: don't fail on custom groups defined by eval

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 64543 in the body.
You can then email your comments to 64543 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: sbaugh <at> catern.com
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] package-report-bug: don't fail on custom groups defined by
 eval
Date: Sun, 09 Jul 2023 17:08:18 +0000 (UTC)

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

Tags: patch


Previously we just assumed that the car of an element of
custom-current-group-alist was a filename.  But actually it can be nil
if a custom group was defined by just evaling Lisp.

* lisp/emacs-lisp/package.el (package-report-bug): Don't fail when a
custom group was defined by eval.

In GNU Emacs 29.0.92 (build 13, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2023-07-04 built on earth
Repository revision: 5ef6e74202e2bdae5999913bff527a4a16d9176e
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: NixOS 23.05 (Stoat)

Configured using:
 'configure --cache-file=config.cache --with-x-toolkit=lucid
 --with-tree-sitter --with-xinput2 CC=gcc PKG_CONFIG=pkg-config
 PKG_CONFIG_PATH=/nix/store/s3r15m8wbl4wqk4khqlf41ikryhjm1bi-file-5.44-dev/lib/pkgconfig:/nix/store/f9jbn419h46c78z1pi49yn9a8742b0ql-gnutls-3.8.0-dev/lib/pkgconfig:/nix/store/knq0pv08wm4dins7m4jh0n7cv7pjvdjr-nettle-3.9.1-dev/lib/pkgconfig:/nix/store/dy8p07vrrhdgpnl45xz9c0k0didbikdh-gmp-with-cxx-6.2.1-dev/lib/pkgconfig:/nix/store/6hkdabzyqhyq5ypq4c9b2cibr1d1zg1s-harfbuzz-7.3.0-dev/lib/pkgconfig:/nix/store/hyns944pqgblw4icskylvlpm5krmfvcr-graphite2-1.3.14-dev/lib/pkgconfig:/nix/store/08cdp9vgvy023ysfa2y01gzsm2jv6phx-jansson-2.14/lib/pkgconfig:/nix/store/nqlbk40lh7igs61l77dwgdkn8dc2akcm-libxml2-2.10.4-dev/lib/pkgconfig:/nix/store/b3axl73v3yvqqx7g47njqb5wzxvm280p-zlib-1.2.13-dev/lib/pkgconfig:/nix/store/3f2rc4inlcxmq11718qmz94v2rpybw70-ncurses-6.4-dev/lib/pkgconfig:/nix/store/bxy745kyb1fwhpfkiaaz3wgvpkpvwcpq-dbus-1.14.8-dev/lib/pkgconfig:/nix/store/9714v7c4cgpm4yqcyqk6n9xw9iq3a1bs-expat-2.5.0-dev/lib/pkgconfig:/nix/store/zzi7pcadidqh798yddxv6pwdbwpkikma-libselinux-3.3-dev/lib/pkgconfig:/nix/store/w14j7y5nl14vy4ikcivss35jmrqq3fxj-libotf-0.9.16-dev/lib/pkgconfig:/nix/store/arhk7hsch4scyv6m24fw03yq6wq5wbbx-m17n-lib-1.8.2/lib/pkgconfig:/nix/store/1jbbrny8xcjb68lb5m30cvxycfkyhvsv-sqlite-3.42.0-dev/lib/pkgconfig:/nix/store/5vx779yqkxaysv48gicwlgv0ippbrhc4-systemd-253.5-dev/lib/pkgconfig:/nix/store/5vx779yqkxaysv48gicwlgv0ippbrhc4-systemd-253.5-dev/share/pkgconfig:/nix/store/djifahvk3qp06ssqxv6gy1ixdnnypr9s-tree-sitter-0.20.8/lib/pkgconfig:/nix/store/74aasy1d2r5y27zn68cs1rxwy1llzn05-libwebp-1.3.0/lib/pkgconfig:/nix/store/8sk7bp89iwb4gw96fq6xakb6lcy2x52n-Xaw3d-1.6.3/lib/pkgconfig:/nix/store/ppvb3ha8148am3ajnzxnm6i3ri38c01n-libXmu-1.1.3-dev/lib/pkgconfig:/nix/store/jyxf8cjbj3nzh00x48nfram79i63chdi-libX11-1.8.6-dev/lib/pkgconfig:/nix/store/zk9v0nr5zdfi1ybkhcfifmxsng7hfl23-xorgproto-2021.5/share/pkgconfig:/nix/store/3q1k18v8aa6mxs538bha4ry0mp3m321l-libxcb-1.14-dev/lib/pkgconfig:/nix/store/hcscz68zvfk1skyb25wrnha959f6hhrc-libXt-1.2.1-dev/lib/pkgconfig:/nix/store/kl55wj6qc3v481jsgvzm5w2csnhm84zf-libSM-1.2.3-dev/lib/pkgconfig:/nix/store/s3f67kvsn55rxp2rc98xv0hkq364yci1-libICE-1.0.10-dev/lib/pkgconfig:/nix/store/rsw4ri8025jgln8vpsrmg82bzgbcw3zr-cairo-1.16.0-dev/lib/pkgconfig:/nix/store/jir0rqbcy0d9qr9kf5cwf2yphql4ykyw-fontconfig-2.14.2-dev/lib/pkgconfig:/nix/store/n2g3xblaz1k4civv1z6hhm1nsmp3m17p-freetype-2.13.0-dev/lib/pkgconfig:/nix/store/isbmyzm2shmp0wsjr4cy45v2i58h2zvw-bzip2-1.0.8-dev/lib/pkgconfig:/nix/store/bl2qwy78jr2sqm260imgxmd5dzhjqvag-brotli-1.0.9-dev/lib/pkgconfig:/nix/store/z96jh9ag5b3565lwwb5chjb9bfp5i2qv-libpng-apng-1.6.39-dev/lib/pkgconfig:/nix/store/jjd4z18grhky6lh8n463v648nnf5628b-pixman-0.42.2/lib/pkgconfig:/nix/store/qd14wrazwcspjv3q65vgh35pl7b8nifq-libXext-1.3.4-dev/lib/pkgconfig:/nix/store/gj8i21xx87ip9b971j2d1m0rmrzyhbir-libXau-1.0.9-dev/lib/pkgconfig:/nix/store/4gpinwwdqhi927xkrfpr1hvdd56baxgk-libXrender-0.9.10-dev/lib/pkgconfig:/nix/store/d1jbygs6hcn6dysk706i9zf07yd18wmr-xcb-util-0.4.1-dev/lib/pkgconfig:/nix/store/hdc4ika0mb1cv0cf6dchwxbr004rc50i-glib-2.76.3-dev/lib/pkgconfig:/nix/store/wxyh848a6xcqy2v8727vcwspri53pqwi-libffi-3.4.4-dev/lib/pkgconfig:/nix/store/42jx72681qzliic0xsjhvx24cil2gapk-libGL-1.6.0-dev/lib/pkgconfig:/nix/store/b9lmdkxpvgkj6zc956fvhshzisqpi767-libglvnd-1.6.0-dev/lib/pkgconfig:/nix/store/gff29sbhg1gcw969mpm5rb693kj5v18w-libXaw-1.0.14-dev/lib/pkgconfig:/nix/store/776xijk8rsb1b4c0dsxwq0k82bvm7mm9-libXpm-3.5.15-dev/lib/pkgconfig:/nix/store/qizdmm43xi65mdngal8bpbpqcdc8290d-libjpeg-turbo-2.1.5.1-dev/lib/pkgconfig:/nix/store/db7ix62fx4nvr9j1fjdvnznl2npff4pr-librsvg-2.55.1-dev/lib/pkgconfig:/nix/store/q0hg0951w1dv9y40m9ggln8phwil6lxc-gdk-pixbuf-2.42.10-dev/lib/pkgconfig:/nix/store/34rr5nvgljsc4bi3mxjxg8abmjr1f7hn-libtiff-4.5.0-dev/lib/pkgconfig:/nix/store/zwkr4kjcjs213pw9mhzi46bzlw6qwxzq-libdeflate-1.18/lib/pkgconfig:/nix/store/6na552yzwml88j8g5vqf5h9ir3vw8myi-xz-5.4.3-dev/lib/pkgconfig'

[0001-package-report-bug-don-t-fail-on-custom-groups-defin.patch (text/patch, attachment)]

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: sbaugh <at> catern.com
Cc: 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Mon, 10 Jul 2023 14:13:23 +0000

sbaugh <at> catern.com writes:

Thanks,

> From: Spencer Baugh <sbaugh <at> catern.com>
> Date: Sun, 9 Jul 2023 12:59:50 -0400
> Subject: [PATCH] package-report-bug: don't fail on custom groups defined by
>  eval
>
> Previously we just assumed that the car of an element of
> custom-current-group-alist was a filename.  But actually it can be nil
> if a custom group was defined by just evaling Lisp.

Where is this behaviour documented?  I couldn't reproduce it with a
simple experiment.

> * lisp/emacs-lisp/package.el (package-report-bug): Don't fail when a
> custom group was defined by eval.
> ---
>  lisp/emacs-lisp/package.el | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 3e6acd9b388..f67e99e04b5 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -4642,6 +4642,7 @@ package-report-bug
>                     (boundp (car ent))
>                     (not (eq (custom--standard-value (car ent))
>                              (default-toplevel-value (car ent))))
> +                   (car group)

If you are checking for (car group), when do this in the loop instead
of wrapping the entire `dolist'?

>                     (file-in-directory-p (car group) (package-desc-dir desc)))
>            (push (car ent) vars))))
>      (dlet ((reporter-prompt-for-summary-p t))

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

From: sbaugh <at> catern.com
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Wed, 12 Jul 2023 19:26:13 +0000 (UTC)

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

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

> sbaugh <at> catern.com writes:
>
> Thanks,
>
>> From: Spencer Baugh <sbaugh <at> catern.com>
>> Date: Sun, 9 Jul 2023 12:59:50 -0400
>> Subject: [PATCH] package-report-bug: don't fail on custom groups defined by
>>  eval
>>
>> Previously we just assumed that the car of an element of
>> custom-current-group-alist was a filename.  But actually it can be nil
>> if a custom group was defined by just evaling Lisp.
>
> Where is this behaviour documented?  I couldn't reproduce it with a
> simple experiment.

To reproduce:
M-: (defgroup mygroup nil "my group") RET

I don't think it's documented anywhere?  custom-current-group-alist is
not documented so this wouldn't be either.

>> * lisp/emacs-lisp/package.el (package-report-bug): Don't fail when a
>> custom group was defined by eval.
>> ---
>>  lisp/emacs-lisp/package.el | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> index 3e6acd9b388..f67e99e04b5 100644
>> --- a/lisp/emacs-lisp/package.el
>> +++ b/lisp/emacs-lisp/package.el
>> @@ -4642,6 +4642,7 @@ package-report-bug
>>                     (boundp (car ent))
>>                     (not (eq (custom--standard-value (car ent))
>>                              (default-toplevel-value (car ent))))
>> +                   (car group)
>
> If you are checking for (car group), when do this in the loop instead
> of wrapping the entire `dolist'?

Good point.  I was just copying the last condition in the and.  Lifted
them both out of the loop:

[0001-Support-transforming-the-dirname-used-by-uniquify.patch (text/x-patch, inline)]

From 2873d4c482acfd1ced749d593fd512c408e0a578 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> catern.com>
Date: Sun, 9 Jul 2023 22:21:03 -0400
Subject: [PATCH] Support transforming the dirname used by uniquify

By transforming the dirname, we can add additional information to use
during uniquifying.  A basic one: uniquifying buffer names based on
the project name.

* lisp/progmodes/project.el (project-uniquify-dirname-transform): Add.
* lisp/uniquify.el (uniquify-dirname-transform-default)
(uniquify-dirname-transform): Add. (bug#62621)
(uniquify-rationalize-file-buffer-names, uniquify-buffer-file-name):
Use uniquify-dirname-transform.
* test/lisp/uniquify-tests.el (uniquify-home,
uniquify-project-transform): Add tests.
---
 lisp/progmodes/project.el   | 12 ++++++++++++
 lisp/uniquify.el            | 26 +++++++++++++++++++++-----
 test/lisp/uniquify-tests.el | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index d482cc24d70..78f9fb410c1 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1835,5 +1835,17 @@ project-switch-project
     (let ((project-current-directory-override dir))
       (call-interactively command))))
 
+;;;###autoload
+(defun project-uniquify-dirname-transform (dirname)
+  "Include `project-name' in DIRNAME if in a project."
+  (if-let (proj (project-current nil dirname))
+      (let ((root (project-root proj)))
+        (expand-file-name
+         (file-name-concat
+          (file-name-directory root)
+          (project-name proj)
+          (file-relative-name dirname root))))
+    dirname))
+
 (provide 'project)
 ;;; project.el ends here
diff --git a/lisp/uniquify.el b/lisp/uniquify.el
index d1ca455b673..b2f28a10d51 100644
--- a/lisp/uniquify.el
+++ b/lisp/uniquify.el
@@ -168,6 +168,20 @@ uniquify-list-buffers-directory-modes
 That means that when `buffer-file-name' is set to nil, `list-buffers-directory'
 contains the name of the directory which the buffer is visiting.")
 
+(defun uniquify-dirname-transform-default (dirname)
+  "Simply return DIRNAME unchanged."
+  dirname)
+
+(defcustom uniquify-dirname-transform #'uniquify-dirname-transform-default
+  "A function to transform the dirname used to uniquify a buffer.
+
+It takes a single argument: the directory of the buffer.  It
+should return a string filename (which does not need to actually
+exist in the filesystem) to use for uniquifying the buffer name."
+  :type '(choice (function-item uniquify-dirname-transform-default)
+                 function)
+  :group 'uniquify)
+
 ;;; Utilities
 
 ;; uniquify-fix-list data structure
@@ -209,7 +223,8 @@ uniquify-rationalize-file-buffer-names
   ;; this buffer.
   (with-current-buffer newbuf (setq uniquify-managed nil))
   (when dirname
-    (setq dirname (expand-file-name (directory-file-name dirname)))
+    (setq dirname (funcall uniquify-dirname-transform
+                           (expand-file-name (directory-file-name dirname))))
     (let ((fix-list (list (uniquify-make-item base dirname newbuf
                                               nil)))
 	  items)
@@ -268,10 +283,11 @@ uniquify-buffer-file-name
 	       (if (memq major-mode uniquify-list-buffers-directory-modes)
 		   list-buffers-directory))))
       (when filename
-	(directory-file-name
-	 (file-name-directory
-	  (expand-file-name
-	   (directory-file-name filename))))))))
+	 (funcall uniquify-dirname-transform
+	          (directory-file-name
+	          (file-name-directory
+	           (expand-file-name
+	            (directory-file-name filename)))))))))
 
 (defun uniquify-rerationalize-w/o-cb (fix-list)
   "Re-rationalize the buffers in FIX-LIST, but ignoring `current-buffer'."
diff --git a/test/lisp/uniquify-tests.el b/test/lisp/uniquify-tests.el
index abd61fa3504..e533c4b644c 100644
--- a/test/lisp/uniquify-tests.el
+++ b/test/lisp/uniquify-tests.el
@@ -88,6 +88,21 @@ uniquify-dirs
                        '("a/dir/" "b/dir/")))
         (mapc #'kill-buffer bufs)))))
 
+(ert-deftest uniquify-home ()
+  "uniquify works, albeit confusingly, in the presence of directories named \"~\""
+  (let (bufs)
+    (save-excursion
+      (push (find-file-noselect "~") bufs)
+      (push (find-file-noselect "./~") bufs)
+      (should (equal (mapcar #'buffer-name bufs)
+                     '("~<test>" "~<>")))
+      (push (find-file-noselect "~/foo") bufs)
+      (push (find-file-noselect "./~/foo") bufs)
+      (should (equal (mapcar #'buffer-name bufs)
+                     '("foo<~>" "foo</nonexistent>" "~<test>" "~<>")))
+      (while bufs
+        (kill-buffer (pop bufs))))))
+
 (ert-deftest uniquify-rename-to-dir ()
   "Giving a buffer a name which matches a directory doesn't rename the buffer"
   (let ((uniquify-buffer-name-style 'forward)
@@ -125,5 +140,23 @@ uniquify-space-prefix
     (should (equal (buffer-name) "| foo"))
     (kill-buffer)))
 
+(require 'project)
+(ert-deftest uniquify-project-transform ()
+  "`project-uniquify-dirname-transform' works"
+  (let ((uniquify-dirname-transform #'project-uniquify-dirname-transform)
+        (project-vc-name "foo1/bar")
+        bufs)
+    (save-excursion
+      (should (file-exists-p "../README"))
+      (push (find-file-noselect "../README") bufs)
+      (push (find-file-noselect "other/README") bufs)
+      (should (equal (mapcar #'buffer-name bufs)
+                     '("README<other>" "README<bar>")))
+      (push (find-file-noselect "foo2/bar/README") bufs)
+      (should (equal (mapcar #'buffer-name bufs)
+                     '("README<foo2/bar>" "README<other>" "README<foo1/bar>")))
+      (while bufs
+        (kill-buffer (pop bufs))))))
+
 (provide 'uniquify-tests)
 ;;; uniquify-tests.el ends here
-- 
2.41.0

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: sbaugh <at> catern.com
Cc: 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Wed, 12 Jul 2023 19:48:43 +0000

sbaugh <at> catern.com writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> sbaugh <at> catern.com writes:
>>
>> Thanks,
>>
>>> From: Spencer Baugh <sbaugh <at> catern.com>
>>> Date: Sun, 9 Jul 2023 12:59:50 -0400
>>> Subject: [PATCH] package-report-bug: don't fail on custom groups defined by
>>>  eval
>>>
>>> Previously we just assumed that the car of an element of
>>> custom-current-group-alist was a filename.  But actually it can be nil
>>> if a custom group was defined by just evaling Lisp.
>>
>> Where is this behaviour documented?  I couldn't reproduce it with a
>> simple experiment.
>
> To reproduce:
> M-: (defgroup mygroup nil "my group") RET

OK, I misunderstood your point, I assumed we were talking about
`defcustom' declarations that were evaluated outside the context of a
file.

> I don't think it's documented anywhere?  custom-current-group-alist is
> not documented so this wouldn't be either.
>
>>> * lisp/emacs-lisp/package.el (package-report-bug): Don't fail when a
>>> custom group was defined by eval.
>>> ---
>>>  lisp/emacs-lisp/package.el | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>>> index 3e6acd9b388..f67e99e04b5 100644
>>> --- a/lisp/emacs-lisp/package.el
>>> +++ b/lisp/emacs-lisp/package.el
>>> @@ -4642,6 +4642,7 @@ package-report-bug
>>>                     (boundp (car ent))
>>>                     (not (eq (custom--standard-value (car ent))
>>>                              (default-toplevel-value (car ent))))
>>> +                   (car group)
>>
>> If you are checking for (car group), when do this in the loop instead
>> of wrapping the entire `dolist'?
>
> Good point.  I was just copying the last condition in the and.  Lifted
> them both out of the loop:
>
> From 2873d4c482acfd1ced749d593fd512c408e0a578 Mon Sep 17 00:00:00 2001
> From: Spencer Baugh <sbaugh <at> catern.com>
> Date: Sun, 9 Jul 2023 22:21:03 -0400
> Subject: [PATCH] Support transforming the dirname used by uniquify

Wrong patch?

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: sbaugh <at> catern.com, 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Wed, 12 Jul 2023 15:56:44 -0400

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

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

> sbaugh <at> catern.com writes:
>> Good point.  I was just copying the last condition in the and.  Lifted
>> them both out of the loop:
>>
>> From 2873d4c482acfd1ced749d593fd512c408e0a578 Mon Sep 17 00:00:00 2001
>> From: Spencer Baugh <sbaugh <at> catern.com>
>> Date: Sun, 9 Jul 2023 22:21:03 -0400
>> Subject: [PATCH] Support transforming the dirname used by uniquify
>
> Wrong patch?
Oops! Here's the right one:

[0001-package-report-bug-don-t-fail-on-custom-groups-defin.patch (text/x-patch, inline)]

From fb31777bcc1935de351eaa35c1ed679c66d24a92 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> catern.com>
Date: Sun, 9 Jul 2023 12:59:50 -0400
Subject: [PATCH] package-report-bug: don't fail on custom groups defined by
 eval

Previously we just assumed that the car of an element of
custom-current-group-alist was a filename.  But actually it can be nil
if a custom group was defined by just evaling Lisp.

* lisp/emacs-lisp/package.el (package-report-bug): Don't fail when a
custom group was defined by eval.
---
 lisp/emacs-lisp/package.el | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 3e6acd9b388..58ca19f7fe2 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -4637,13 +4637,14 @@ package-report-bug
         vars)
     (dolist-with-progress-reporter (group custom-current-group-alist)
         "Scanning for modified user options..."
-      (dolist (ent (get (cdr group) 'custom-group))
-        (when (and (custom-variable-p (car ent))
-                   (boundp (car ent))
-                   (not (eq (custom--standard-value (car ent))
-                            (default-toplevel-value (car ent))))
-                   (file-in-directory-p (car group) (package-desc-dir desc)))
-          (push (car ent) vars))))
+      (when (and (car group)
+                 (file-in-directory-p (car group) (package-desc-dir desc)))
+        (dolist (ent (get (cdr group) 'custom-group))
+          (when (and (custom-variable-p (car ent))
+                     (boundp (car ent))
+                     (not (eq (custom--standard-value (car ent))
+                              (default-toplevel-value (car ent)))))
+            (push (car ent) vars)))))
     (dlet ((reporter-prompt-for-summary-p t))
       (reporter-submit-bug-report maint name vars))))
 
-- 
2.39.3

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: sbaugh <at> catern.com
Cc: philipk <at> posteo.net, 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom groups
 defined by eval
Date: Thu, 13 Jul 2023 07:54:25 +0300

> Cc: 64543 <at> debbugs.gnu.org
> From: sbaugh <at> catern.com
> Date: Wed, 12 Jul 2023 19:26:13 +0000 (UTC)
> 
> +(defun uniquify-dirname-transform-default (dirname)
> +  "Simply return DIRNAME unchanged."
> +  dirname)

If you don't explain the need for its existence in the doc string,
this function looks weirdly redundant.

> +(defcustom uniquify-dirname-transform #'uniquify-dirname-transform-default
> +  "A function to transform the dirname used to uniquify a buffer.

Come to think about it, why not use 'identity as the default value?
Then the above weird function could be deleted.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: sbaugh <at> catern.com, 'Eli Zaretskii' <eliz <at> gnu.org>, 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Fri, 14 Jul 2023 19:44:15 +0000

Spencer Baugh <sbaugh <at> janestreet.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> sbaugh <at> catern.com writes:
>>> Good point.  I was just copying the last condition in the and.  Lifted
>>> them both out of the loop:
>>>
>>> From 2873d4c482acfd1ced749d593fd512c408e0a578 Mon Sep 17 00:00:00 2001
>>> From: Spencer Baugh <sbaugh <at> catern.com>
>>> Date: Sun, 9 Jul 2023 22:21:03 -0400
>>> Subject: [PATCH] Support transforming the dirname used by uniquify
>>
>> Wrong patch?
> Oops! Here's the right one:
>
> From fb31777bcc1935de351eaa35c1ed679c66d24a92 Mon Sep 17 00:00:00 2001
> From: Spencer Baugh <sbaugh <at> catern.com>
> Date: Sun, 9 Jul 2023 12:59:50 -0400
> Subject: [PATCH] package-report-bug: don't fail on custom groups defined by
>  eval
>
> Previously we just assumed that the car of an element of
> custom-current-group-alist was a filename.  But actually it can be nil
> if a custom group was defined by just evaling Lisp.
>
> * lisp/emacs-lisp/package.el (package-report-bug): Don't fail when a
> custom group was defined by eval.
> ---
>  lisp/emacs-lisp/package.el | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 3e6acd9b388..58ca19f7fe2 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -4637,13 +4637,14 @@ package-report-bug
>          vars)
>      (dolist-with-progress-reporter (group custom-current-group-alist)
>          "Scanning for modified user options..."
> -      (dolist (ent (get (cdr group) 'custom-group))
> -        (when (and (custom-variable-p (car ent))
> -                   (boundp (car ent))
> -                   (not (eq (custom--standard-value (car ent))
> -                            (default-toplevel-value (car ent))))
> -                   (file-in-directory-p (car group) (package-desc-dir desc)))
> -          (push (car ent) vars))))
> +      (when (and (car group)
> +                 (file-in-directory-p (car group) (package-desc-dir desc)))
> +        (dolist (ent (get (cdr group) 'custom-group))
> +          (when (and (custom-variable-p (car ent))
> +                     (boundp (car ent))
> +                     (not (eq (custom--standard-value (car ent))
> +                              (default-toplevel-value (car ent)))))
> +            (push (car ent) vars)))))
>      (dlet ((reporter-prompt-for-summary-p t))
>        (reporter-submit-bug-report maint name vars))))

LGTM.

Eli, would it be OK to push this to emacs-29, as this is a bug in the
existing code?  Most of the changes are indentation/whitespace changes
anyway.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: sbaugh <at> janestreet.com, 64543 <at> debbugs.gnu.org, sbaugh <at> catern.com
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sat, 15 Jul 2023 08:49:21 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: sbaugh <at> catern.com,  64543 <at> debbugs.gnu.org, "'Eli Zaretskii'" <eliz <at> gnu.org>
> Date: Fri, 14 Jul 2023 19:44:15 +0000
> 
> Eli, would it be OK to push this to emacs-29, as this is a bug in the
> existing code?  Most of the changes are indentation/whitespace changes
> anyway.

I don't understand the cases in which this bug happens, and so cannot
make the decision.  The entire discussion of this bug never described
such a case, AFAICT.  (Also, the commit log message could use some
loving care: what does it mean when it says "Previously"?)

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> janestreet.com, 64543 <at> debbugs.gnu.org, sbaugh <at> catern.com
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sat, 15 Jul 2023 07:40:01 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: sbaugh <at> catern.com,  64543 <at> debbugs.gnu.org, "'Eli Zaretskii'" <eliz <at> gnu.org>
>> Date: Fri, 14 Jul 2023 19:44:15 +0000
>> 
>> Eli, would it be OK to push this to emacs-29, as this is a bug in the
>> existing code?  Most of the changes are indentation/whitespace changes
>> anyway.
>
> I don't understand the cases in which this bug happens, and so cannot
> make the decision.  

Spencer gave this example:

  >> Previously we just assumed that the car of an element of
  >> custom-current-group-alist was a filename.  But actually it can be nil
  >> if a custom group was defined by just evaling Lisp.
  >
  > Where is this behaviour documented?  I couldn't reproduce it with a
  > simple experiment.

  To reproduce:
  M-: (defgroup mygroup nil "my group") RET

The patch would ensure that if groups like these are defined (which
might happen by mistake), then `package-report-bug' will remain robust
and not fail due to a unrelated issue.

>                     The entire discussion of this bug never described
> such a case, AFAICT.  (Also, the commit log message could use some
> loving care: what does it mean when it says "Previously"?)

How about

--8<---------------cut here---------------start------------->8---
* lisp/emacs-lisp/package.el (package-report-bug): Do not assume that
every entry in 'custom-current-group-alist' has a non-nil entry for a
filename.

It is possible for a group to not be associated with any file, e.g. when
a 'defgroup' form is evaluated using 'eval-expression'.  (bug#64543)
--8<---------------cut here---------------end--------------->8---

A related discussion might be if `custom-declare-group' could be
improved, by not just consulting `load-file-name', but using
`macroexp-file-name'.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: sbaugh <at> janestreet.com, 64543 <at> debbugs.gnu.org, sbaugh <at> catern.com
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sat, 15 Jul 2023 11:57:06 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: sbaugh <at> janestreet.com,  sbaugh <at> catern.com,  64543 <at> debbugs.gnu.org
> Date: Sat, 15 Jul 2023 07:40:01 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> Eli, would it be OK to push this to emacs-29, as this is a bug in the
> >> existing code?  Most of the changes are indentation/whitespace changes
> >> anyway.
> >
> > I don't understand the cases in which this bug happens, and so cannot
> > make the decision.  
> 
> Spencer gave this example:
> 
>   >> Previously we just assumed that the car of an element of
>   >> custom-current-group-alist was a filename.  But actually it can be nil
>   >> if a custom group was defined by just evaling Lisp.
>   >
>   > Where is this behaviour documented?  I couldn't reproduce it with a
>   > simple experiment.
> 
>   To reproduce:
>   M-: (defgroup mygroup nil "my group") RET
> 
> The patch would ensure that if groups like these are defined (which
> might happen by mistake), then `package-report-bug' will remain robust
> and not fail due to a unrelated issue.

Is this case important enough to make this change so late in the
pretest?  Spencer, how did you bump into this situation in Real Life?

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

From: Spencer Baugh <sbaugh <at> catern.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> janestreet.com, Philip Kaludercic <philipk <at> posteo.net>,
 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sat, 15 Jul 2023 09:12:48 +0000 (UTC)

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> catern.com>
Cc: sbaugh <at> janestreet.com, philipk <at> posteo.net, 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sat, 15 Jul 2023 12:23:21 +0300

> Date: Sat, 15 Jul 2023 09:12:48 +0000 (UTC)
> From: Spencer Baugh <sbaugh <at> catern.com>
> Cc: Philip Kaludercic <philipk <at> posteo.net>, sbaugh <at> janestreet.com,
> 	64543 <at> debbugs.gnu.org
> 
> On Jul 15, 2023 04:57, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  >   >> Previously we just assumed that the car of an element of 
>  >   >> custom-current-group-alist was a filename.  But actually it can be nil 
>  >   >> if a custom group was defined by just evaling Lisp. 
>  >   > 
>  >   > Where is this behaviour documented?  I couldn't reproduce it with a 
>  >   > simple experiment. 
>  > 
>  >   To reproduce: 
>  >   M-: (defgroup mygroup nil "my group") RET 
>  > 
>  > The patch would ensure that if groups like these are defined (which 
>  > might happen by mistake), then `package-report-bug' will remain robust 
>  > and not fail due to a unrelated issue. 
> 
>  Is this case important enough to make this change so late in the 
>  pretest?  Spencer, how did you bump into this situation in Real Life?
> 
> I evaled a buffer containing a defgroup with C-c C-e (or maybe just a region?)

Why did you do that?

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Spencer Baugh <sbaugh <at> catern.com>, 64543 <at> debbugs.gnu.org,
 sbaugh <at> janestreet.com
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sat, 15 Jul 2023 10:05:13 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sat, 15 Jul 2023 09:12:48 +0000 (UTC)
>> From: Spencer Baugh <sbaugh <at> catern.com>
>> Cc: Philip Kaludercic <philipk <at> posteo.net>, sbaugh <at> janestreet.com,
>> 	64543 <at> debbugs.gnu.org
>> 
>> On Jul 15, 2023 04:57, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> 
>>  >   >> Previously we just assumed that the car of an element of 
>>  >   >> custom-current-group-alist was a filename.  But actually it can be nil 
>>  >   >> if a custom group was defined by just evaling Lisp. 
>>  >   > 
>>  >   > Where is this behaviour documented?  I couldn't reproduce it with a 
>>  >   > simple experiment. 
>>  > 
>>  >   To reproduce: 
>>  >   M-: (defgroup mygroup nil "my group") RET 
>>  > 
>>  > The patch would ensure that if groups like these are defined (which 
>>  > might happen by mistake), then `package-report-bug' will remain robust 
>>  > and not fail due to a unrelated issue. 
>> 
>>  Is this case important enough to make this change so late in the 
>>  pretest?  Spencer, how did you bump into this situation in Real Life?
>> 
>> I evaled a buffer containing a defgroup with C-c C-e (or maybe just a region?)
>
> Why did you do that?

Evaluating an entire buffer with C-c C-e is not unreasonable, but it
will not bind `load-file-name', which `load-file' would have done, but
there is no binding for that.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: sbaugh <at> catern.com, 64543 <at> debbugs.gnu.org, sbaugh <at> janestreet.com
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sat, 15 Jul 2023 13:23:16 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: Spencer Baugh <sbaugh <at> catern.com>,  sbaugh <at> janestreet.com,
>   64543 <at> debbugs.gnu.org
> Date: Sat, 15 Jul 2023 10:05:13 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> I evaled a buffer containing a defgroup with C-c C-e (or maybe just a region?)
> >
> > Why did you do that?
> 
> Evaluating an entire buffer with C-c C-e is not unreasonable, but it
> will not bind `load-file-name', which `load-file' would have done, but
> there is no binding for that.

<Shrug> it sounds strange to me.

But okay, please install on emacs-29 with the improved log message,
and thanks.

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

From: sbaugh <at> catern.com
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: sbaugh <at> janestreet.com, Eli Zaretskii <eliz <at> gnu.org>, 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sat, 15 Jul 2023 17:45:12 +0000 (UTC)

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

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> Date: Sat, 15 Jul 2023 09:12:48 +0000 (UTC)
>>> From: Spencer Baugh <sbaugh <at> catern.com>
>>> Cc: Philip Kaludercic <philipk <at> posteo.net>, sbaugh <at> janestreet.com,
>>> 	64543 <at> debbugs.gnu.org
>>> 
>>> On Jul 15, 2023 04:57, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>> 
>>>  >   >> Previously we just assumed that the car of an element of 
>>>  >   >> custom-current-group-alist was a filename.  But actually it
>>>  >   >> can be nil
>>>  >   >> if a custom group was defined by just evaling Lisp. 
>>>  >   > 
>>>  >   > Where is this behaviour documented?  I couldn't reproduce it with a 
>>>  >   > simple experiment. 
>>>  > 
>>>  >   To reproduce: 
>>>  >   M-: (defgroup mygroup nil "my group") RET 
>>>  > 
>>>  > The patch would ensure that if groups like these are defined (which 
>>>  > might happen by mistake), then `package-report-bug' will remain robust 
>>>  > and not fail due to a unrelated issue. 
>>> 
>>>  Is this case important enough to make this change so late in the 
>>>  pretest?  Spencer, how did you bump into this situation in Real Life?
>>> 
>>> I evaled a buffer containing a defgroup with C-c C-e (or maybe just
>>> a region?)
>>
>> Why did you do that?
>
> Evaluating an entire buffer with C-c C-e is not unreasonable, but it
> will not bind `load-file-name', which `load-file' would have done, but
> there is no binding for that.

Maybe we should add a binding for load-file?  Or maybe C-c C-e should
bind load-file-name if the region is not active?

Btw, just curious: how do others test modifications they make to Lisp
files?  I usually make a couple changes to several functions or
variables and then reloading the whole file is easier than going defun
by defun.

Message #52 received at 64543-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> catern.com, 64543-done <at> debbugs.gnu.org, sbaugh <at> janestreet.com
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sat, 15 Jul 2023 23:33:40 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: Spencer Baugh <sbaugh <at> catern.com>,  sbaugh <at> janestreet.com,
>>   64543 <at> debbugs.gnu.org
>> Date: Sat, 15 Jul 2023 10:05:13 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> I evaled a buffer containing a defgroup with C-c C-e (or maybe
>> > just a region?)
>> >
>> > Why did you do that?
>> 
>> Evaluating an entire buffer with C-c C-e is not unreasonable, but it
>> will not bind `load-file-name', which `load-file' would have done, but
>> there is no binding for that.
>
> <Shrug> it sounds strange to me.
>
> But okay, please install on emacs-29 with the improved log message,
> and thanks.

Done.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: sbaugh <at> catern.com
Cc: sbaugh <at> janestreet.com, Eli Zaretskii <eliz <at> gnu.org>, 64543 <at> debbugs.gnu.org
Subject: Re: bug#64543: [PATCH] package-report-bug: don't fail on custom
 groups defined by eval
Date: Sun, 16 Jul 2023 12:33:59 +0000

sbaugh <at> catern.com writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>>> Date: Sat, 15 Jul 2023 09:12:48 +0000 (UTC)
>>>> From: Spencer Baugh <sbaugh <at> catern.com>
>>>> Cc: Philip Kaludercic <philipk <at> posteo.net>, sbaugh <at> janestreet.com,
>>>> 	64543 <at> debbugs.gnu.org
>>>> 
>>>> On Jul 15, 2023 04:57, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>>> 
>>>>  >   >> Previously we just assumed that the car of an element of 
>>>>  >   >> custom-current-group-alist was a filename.  But actually it
>>>>  >   >> can be nil
>>>>  >   >> if a custom group was defined by just evaling Lisp. 
>>>>  >   > 
>>>>  >   > Where is this behaviour documented?  I couldn't reproduce it with a 
>>>>  >   > simple experiment. 
>>>>  > 
>>>>  >   To reproduce: 
>>>>  >   M-: (defgroup mygroup nil "my group") RET 
>>>>  > 
>>>>  > The patch would ensure that if groups like these are defined (which 
>>>>  > might happen by mistake), then `package-report-bug' will remain robust 
>>>>  > and not fail due to a unrelated issue. 
>>>> 
>>>>  Is this case important enough to make this change so late in the 
>>>>  pretest?  Spencer, how did you bump into this situation in Real Life?
>>>> 
>>>> I evaled a buffer containing a defgroup with C-c C-e (or maybe just
>>>> a region?)
>>>
>>> Why did you do that?
>>
>> Evaluating an entire buffer with C-c C-e is not unreasonable, but it
>> will not bind `load-file-name', which `load-file' would have done, but
>> there is no binding for that.
>
> Maybe we should add a binding for load-file?  Or maybe C-c C-e should
> bind load-file-name if the region is not active?

I am not familiar enough with the intention between distinguishing
evaluation and loading to comment on this, or where the distinction
/should/ matter.

> Btw, just curious: how do others test modifications they make to Lisp
> files?  I usually make a couple changes to several functions or
> variables and then reloading the whole file is easier than going defun
> by defun.

I either eval the defuns right after editing them using C-M-x or I'd
infrequently use M-x load-file.

Previous Next

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