bug#64649: 30.0.50; Customize: Comments on face customization not being saved

bug#64649: 30.0.50; Customize: Comments on face customization not being saved

[Mauro Aranda]

This cannot be reproduced with emacs -Q, since it involves saving the
customizations to the custom-file.

So, backup your custom-file, follow these steps with a test user, or
something.  Start Emacs and evaluate the following:

(defface foo '((t nil))
   "...")

Then: M-x customize-face RET foo
Edit at least an attribute, like the foreground color, for example.
Action the State button and "Add Comment"
Add a comment in the comment widget, like "Test"
Action the State button and "Save for Future Sessions"
Now visit the custom file: C-x C-f CUSTOM-FILE
Search for the foo customization:  '(foo ((t (:foreground "gray"))))
The "Test" comment isn't saved.

This used to work in Emacs 27, but stopped working in Emacs 28.

I think the following commit introduced this regression:

commit a4c07bc8c8201a620c4365c1d0d2cb814cc677a9
Author: Alan Mackenzie <acm@muc.de>
Date:   Fri May 1 12:23:15 2020 +0000

     Protect non-selected face spec components in custimize-face.  Fixes 
bug #40866

     * lisp/cus-edit.el (custom-face-save): If the current face widget 
is only
     displaying part of the face spec, temporarily set it to "display" 
the whole
     spec around the call to custom-face-mark-to-save.



In GNU Emacs 30.0.50 (build 27, x86_64-pc-linux-gnu, GTK+ Version
  3.24.33, cairo version 1.16.0) of 2023-07-14 built on tbb-desktop
Repository revision: e27cb447a8ceeee02428840baa29d122d2b6bd56
Repository branch: bug64347-eli
Windowing system distributor 'The X.Org Foundation', version 11.0.12201001
System Description: Ubuntu 22.04.2 LTS

Configured using:
  'configure --with-mailutils'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LC_MONETARY: es_AR.UTF-8
   value of $LC_NUMERIC: es_AR.UTF-8
   value of $LC_TIME: es_AR.UTF-8
   value of $LANG: en_US.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   auto-insert-mode: t
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   column-number-mode: t
   line-number-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t

Load-path shadows:
~/code/emacs/hiddenquote/hiddenquote/hiddenquote hides 
/home/tbb/.emacs.d/elpa/hiddenquote-20210205.1401/hiddenquote
/home/tbb/.emacs.d/elpa/transient-20221019.941/transient hides 
/home/tbb/code/emacs/emacs/lisp/transient

Features:
(shadow mail-extr emacsbug message yank-media puny rfc822 mml mml-sec
epa derived epg epg-config mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail
solarized-dark-high-contrast-theme solarized-palettes solarized
solarized-faces color autoinsert init-perl init-web company-web dash
company-css web-completion-data company edmacro kmacro pcase init-ext
init-emms dbus emms-librefm-stream emms-librefm-scrobbler
emms-playlist-limit emms-volume easy-mmode emms-volume-mixerctl
emms-volume-pulse emms-volume-amixer emms-i18n emms-history emms-score
emms-stream-info emms-metaplaylist-mode emms-bookmarks emms-cue
emms-mode-line-icon emms-browser sort emms-playlist-sort
emms-last-played emms-player-xine emms-player-mpd tq emms-playing-time
emms-lyrics emms-url emms-streams emms-show-all emms-tag-editor
emms-mark emms-mode-line emms-cache emms-info-exiftool emms-info-tinytag
emms-info-metaflac emms-info-opusinfo emms-info-ogginfo
emms-info-mp3info emms-info emms-later-do emms-playlist-mode
emms-player-vlc advice emms-player-mpv emms-player-mplayer
emms-player-simple emms-source-playlist emms-source-file locate dired
dired-loaddefs emms-setup emms emms-compat init-gnus gnus nnheader
gnus-util text-property-search range init-org s ox-odt rng-loc rng-uri
rng-parse rng-match rng-dt rng-util rng-pttrn nxml-parse nxml-ns
nxml-enc xmltok nxml-util ox-latex ox-icalendar org-agenda ox-html table
ox-ascii ox-publish ox org-element org-persist xdg org-id org-refile
avl-tree org-clock org ob ob-tangle ob-ref ob-lob ob-table ob-exp
org-macro org-src ob-comint org-pcomplete org-list org-footnote
org-faces org-entities noutline outline icons ob-emacs-lisp ob-core
ob-eval org-cycle org-table ol rx org-fold org-fold-core org-keys oc
org-loaddefs find-func cal-menu calendar cal-loaddefs org-version
org-compat org-macs format-spec init-social newsticker newst-treeview
tree-widget wid-edit newst-plainview newst-reader newst-ticker
newst-backend iso8601 time-date xml init-octave init-rmail undigest
rmail rfc6068 rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
init-vc vc vc-dispatcher init-cc-mode init-c init-yasnippet init-eshell
eshell esh-cmd generator esh-ext esh-opt esh-proc esh-io esh-arg
pcomplete comint ansi-osc ansi-color ring esh-module esh-groups esh-util
files-x init-lisp init-elisp init-global finder-inf init-package
750words-autoloads adjust-parens-autoloads auctex-autoloads tex-site
cider-autoloads epkg-autoloads closql-autoloads git-modes-autoloads
inf-clojure-autoloads clojure-mode-autoloads magit-autoloads
git-commit-autoloads magit-section-autoloads parseedn-autoloads
parseclj-autoloads pdf-tools-autoloads perl-doc-autoloads
queue-autoloads sesman-autoloads slime-autoloads swift-mode-autoloads
uniquify-files-autoloads with-editor-autoloads dash-autoloads
transient-autoloads info compat-autoloads package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo gtk x-toolkit xinput2 x multi-tty move-toolbar
make-network-process emacs)

Memory information:
((conses 16 461348 66691) (symbols 48 31470 0)
  (strings 32 151984 5310) (string-bytes 1 4227883) (vectors 16 55630)
  (vector-slots 8 1370848 173062) (floats 8 377 4610)
  (intervals 56 450 0) (buffers 984 10))

bug#64649: 30.0.50; Customize: Comments on face customization not being saved

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 206 bytes --]

I attach a patch to fix this.  The commit that introduced this
regression showed that we weren't taking care of preserving the comments
when redrawing either the custom-face or the custom-variable widget.

[-- Attachment #2: 0001-Preserve-comments-when-redrawing-a-widget-Bug-64649.patch --]
[-- Type: text/x-patch, Size: 4015 bytes --]

From d77851ba8b434697a7e5a797e9115fe008b4ecf5 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sat, 15 Jul 2023 18:54:14 -0300
Subject: [PATCH] Preserve comments when redrawing a widget (Bug#64649)

* lisp/cus-edit.el (custom-comment-preserve): New function.
(custom-redraw): Use it.
(custom-comment-create): Make sure :comment-shown is set to t if the
comment widget gets created.
(custom-face-value-create, custom-variable-value-create): Recreate the
custom-comment widget with the preserved value, if any.
---
 lisp/cus-edit.el | 45 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index dbef5f47cd6..1c4de92db12 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -2330,6 +2330,7 @@ custom-redraw
 	(from (marker-position (widget-get widget :from)))
 	(to (marker-position (widget-get widget :to))))
     (save-excursion
+      (custom-comment-preserve widget)
       (widget-value-set widget (widget-value widget))
       (custom-redraw-magic widget))
     (when (and (>= pos from) (<= pos to))
@@ -2509,7 +2510,9 @@ custom-comment-create
   (let* ((null-comment (equal "" (widget-value widget))))
     (if (or (widget-get (widget-get widget :parent) :comment-shown)
 	    (not null-comment))
-	(widget-default-create widget)
+        (progn
+          (widget-default-create widget)
+          (widget-put (widget-get widget :parent) :comment-shown t))
       ;; `widget-default-delete' expects markers in these slots --
       ;; maybe it shouldn't.
       (widget-put widget :from (point-marker))
@@ -2542,6 +2545,14 @@ custom-comment-invisible-p
     (and (equal "" val)
 	 (not (widget-get widget :comment-shown)))))
 
+;; This is useful when we want to redraw a widget, but we want to preserve
+;; edits made by the user in the comment widget.  (See Bug#64649)
+(defun custom-comment-preserve (widget)
+  "Preserve the comment that belongs to WIDGET."
+  (when (widget-get widget :comment-shown)
+    (let ((comment-widget (widget-get widget :comment-widget)))
+      (widget-put comment-widget :value (widget-value comment-widget)))))
+
 ;;; The `custom-variable' Widget.
 
 (defface custom-variable-obsolete
@@ -2821,12 +2832,16 @@ custom-variable-value-create
 
       ;; The comment field
       (unless (eq state 'hidden)
-	(let* ((comment (get symbol 'variable-comment))
-	       (comment-widget
-		(widget-create-child-and-convert
-		 widget 'custom-comment
-		 :parent widget
-		 :value (or comment ""))))
+        (let ((comment-widget
+               (widget-create-child-and-convert
+                widget 'custom-comment
+                :parent widget
+                :value (or
+                        (and
+                         (widget-get widget :comment-shown)
+                         (widget-value (widget-get widget :comment-widget)))
+                        (get symbol 'variable-comment)
+                        ""))))
 	  (widget-put widget :comment-widget comment-widget)
 	  ;; Don't push it !!! Custom assumes that the first child is the
 	  ;; value one.
@@ -3831,12 +3846,16 @@ custom-face-value-create
 	 widget :visibility-widget 'custom-visibility)
 	;; The comment field
 	(unless hiddenp
-	  (let* ((comment (get symbol 'face-comment))
-		 (comment-widget
-		  (widget-create-child-and-convert
-		   widget 'custom-comment
-		   :parent widget
-		   :value (or comment ""))))
+	  (let ((comment-widget
+                 (widget-create-child-and-convert
+                  widget 'custom-comment
+                  :parent widget
+                  :value (or
+                          (and
+                           (widget-get widget :comment-shown)
+                           (widget-value (widget-get widget :comment-widget)))
+                          (get symbol 'face-comment)
+                          ""))))
 	    (widget-put widget :comment-widget comment-widget)
 	    (push comment-widget children))))
 
-- 
2.34.1

bug#64649: 30.0.50; Customize: Comments on face customization not being saved

[Eli Zaretskii]

> Date: Sat, 15 Jul 2023 18:59:29 -0300
> From: Mauro Aranda <maurooaranda@gmail.com>
> 
> I attach a patch to fix this.  The commit that introduced this
> regression showed that we weren't taking care of preserving the comments
> when redrawing either the custom-face or the custom-variable widget.

Thanks, installed on the master branch, and closing the bug.