bug#24727: 25.1; Editing custom theme removes undefined variables and faces

bug#24727: 25.1; Editing custom theme removes undefined variables and faces

[Allen Li]

When using `customize-create-theme' to visit and edit an existing theme,
undefined variables and faces get removed on save (for example, for a
package whose autoload hasn't been triggered yet).

In GNU Emacs 25.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.20.9)
 of 2016-09-18 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe
 -fstack-protector-strong' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

bug#24727: 25.1; Editing custom theme removes undefined variables and faces

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 1226 bytes --]

Allen Li <vianchielfaura@gmail.com> writes:

> When using `customize-create-theme' to visit and edit an existing theme,
> undefined variables and faces get removed on save (for example, for a
> package whose autoload hasn't been triggered yet).
>

To test this, I used the following theme:
(deftheme tbb-test
  "A test theme.")

(custom-theme-set-variables
 'tbb-test
 '(column-number-mode t)
 '(foo-undefined 'foo))

(provide-theme 'tbb-test)

And put it under custom-theme-directory, with name tbb-test-theme.el.
Then:
M-x custom-theme-visit-theme RET tbb-test

And clicked Save Theme.

Then I visited tbb-test-theme.el and confirmed that the entry for
foo-undefined wasn't there anymore.

There is a check in custom-theme-write-variables for a bound symbol, and
a check in custom-theme-write-faces for a known face, and I wonder why,
because that should not be a concern for these functions.

Note that still editing a theme with custom-theme-visit-theme is
somewhat buggy, because if you click to show the variable foo-undefined
you end up with a lot of errors, and you can't hide it back.

But at least removing the checks makes it less rude, and undefined
variables or faces stay in the file.

Here's my proposed patch.

[-- Attachment #1.2: Type: text/html, Size: 1480 bytes --]

[-- Attachment #2: 0001-Do-not-remove-unbound-variables-or-faces-when-modify.patch --]
[-- Type: text/x-patch, Size: 1626 bytes --]

From 2f33fc011b6f514dbd6bcc80aad30c557af92874 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Wed, 26 Aug 2020 10:22:11 -0300
Subject: [PATCH] Do not remove unbound variables or faces when modifying a
 custom-theme

* lisp/cus-theme.el (custom-theme-write-variables
custom-theme-write-faces): Remove check for a bound symbol or for a
face name, so saving a theme does not remove not yet defined variables
or faces.  (Bug#24727).
---
 lisp/cus-theme.el | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lisp/cus-theme.el b/lisp/cus-theme.el
index b0decfe7b7..dc463e05f9 100644
--- a/lisp/cus-theme.el
+++ b/lisp/cus-theme.el
@@ -419,14 +419,13 @@ custom-theme-write-variables
 			    (widget-value child)
 			  ;; Child is null if the widget is closed (hidden).
 			  (car (widget-get widget :shown-value)))))
-	    (when (boundp symbol)
-	      (unless (bolp)
-		(princ "\n"))
-	      (princ " '(")
-	      (prin1 symbol)
-	      (princ " ")
-	      (prin1 (custom-quote value))
-	      (princ ")")))))
+	    (unless (bolp)
+	      (princ "\n"))
+	    (princ " '(")
+	    (prin1 symbol)
+	    (princ " ")
+	    (prin1 (custom-quote value))
+	    (princ ")"))))
       (if (bolp)
 	  (princ " "))
       (princ ")")
@@ -454,7 +453,7 @@ custom-theme-write-faces
 		   ;; Child is null if the widget is closed (hidden).
 		   ((widget-get widget :shown-value))
 		   (t (custom-face-get-current-spec symbol)))))
-	    (when (and (facep symbol) value)
+	    (when value
 	      (princ (if (bolp) " '(" "\n '("))
 	      (prin1 symbol)
 	      (princ " ")
-- 
2.28.0

bug#24727: 25.1; Editing custom theme removes undefined variables and faces

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> There is a check in custom-theme-write-variables for a bound symbol, and
> a check in custom-theme-write-faces for a known face, and I wonder why,
> because that should not be a concern for these functions.

Yes, that seems odd.  I had a short look around at the commit history
here, but I was unable to find an explanation (but I didn't do an
extensive examination).

> But at least removing the checks makes it less rude, and undefined
> variables or faces stay in the file.
>
> Here's my proposed patch.

It makes sense to me (and the old behaviour didn't), so I've applied it
to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no