Preserve user customizations after disabling a theme

* lisp/custom.el (enable-theme): Since we are enabling the theme, bind
custom--inhibit-theme-enable to nil.  Then rely on custom-push-theme
to do the right thing with the theme settings and prior user settings,
instead of manipulating the property here.  This way, when disabling a
theme, we restore user preferences, even when the values were changed
outside of customize.
(disable-theme): Call custom-push-theme instead of handling theme
settings directly.
(custom-push-theme): Avoid another instance of Bug#28904: we don't
need the changed theme when the value recorded for it is going to be
the same as the recorded for the user theme.

* test/lisp/custom-tests.el (custom--test-theme-variables): Get rid of
a portion of the test that will always fail, because the user theme
has priority over every other theme.  Expect the test to pass now that
we preserve user customizations after disabling a theme (bug#34027).

2 files changed, 16 insertions, 13 deletions

diff --git a/lisp/custom.el b/lisp/custom.el
index 7581457ce8d..cc445fe765b 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -907,7 +907,15 @@ See `custom-known-themes' for a list of known themes."
 		     (boundp symbol))
 	    (let ((sv  (get symbol 'standard-value))
 		  (val (symbol-value symbol)))
-	      (unless (and sv (equal (eval (car sv)) val))
+	      (unless (or
+                       ;; We only do this trick if the current value
+                       ;; is different from the standard value.
+                       (and sv (equal (eval (car sv)) val))
+                       ;; And we don't do it if we would end up recording
+                       ;; the same value for the user theme.  This way we avoid
+                       ;; having ((user VALUE) (changed VALUE)).  That would be
+                       ;; useless, because we don't disable the user theme.
+                       (and (eq theme 'user) (equal (custom-quote val) value)))
 		(setq old `((changed ,(custom-quote val))))))))
 	(put symbol prop (cons (list theme value) old)))
       (put theme 'theme-settings
@@ -1368,13 +1376,14 @@ function runs.  To disable other themes, use `disable-theme'."
 		       obarray (lambda (sym) (get sym 'theme-settings)) t))))
   (unless (custom-theme-p theme)
     (error "Undefined Custom theme %s" theme))
-  (let ((settings (get theme 'theme-settings)))
+  (let ((settings (get theme 'theme-settings)) ; '(prop symbol theme value)
+        ;; We are enabling the theme, so don't inhibit enabling it.  (Bug#34027)
+        (custom--inhibit-theme-enable nil))
     ;; Loop through theme settings, recalculating vars/faces.
     (dolist (s settings)
       (let* ((prop (car s))
-	     (symbol (cadr s))
-	     (spec-list (get symbol prop)))
-	(put symbol prop (cons (cddr s) (assq-delete-all theme spec-list)))
+	     (symbol (cadr s)))
+        (custom-push-theme prop symbol theme 'set (nth 3 s))
 	(cond
 	 ((eq prop 'theme-face)
 	  (custom-theme-recalc-face symbol))
@@ -1443,7 +1452,7 @@ See `custom-enabled-themes' for a list of enabled themes."
 	(let* ((prop   (car s))
 	       (symbol (cadr s))
 	       (val (assq-delete-all theme (get symbol prop))))
-	  (put symbol prop val)
+          (custom-push-theme prop symbol theme 'reset)
 	  (cond
 	   ((eq prop 'theme-value)
 	    (custom-theme-recalc-variable symbol))
diff --git a/test/lisp/custom-tests.el b/test/lisp/custom-tests.el
index 766e4844988..7853c84bb69 100644
--- a/test/lisp/custom-tests.el
+++ b/test/lisp/custom-tests.el
@@ -99,7 +99,6 @@
 ;; This is demonstrating bug#34027.
 (ert-deftest custom--test-theme-variables ()
   "Test variables setting with enabling / disabling a custom theme."
-  :expected-result :failed
   ;; We load custom-resources/custom--test-theme.el.
   (let ((custom-theme-load-path
          `(,(expand-file-name "custom-resources" (file-name-directory #$)))))
@@ -115,15 +114,10 @@
     (should (equal custom--test-user-option 'baz))
     (should (equal custom--test-variable 'baz))
 
+    ;; Enable and then disable.
     (enable-theme 'custom--test)
-    ;; The variables have the theme values.
-    (should (equal custom--test-user-option 'bar))
-    (should (equal custom--test-variable 'bar))
-
     (disable-theme 'custom--test)
     ;; The variables should have the changed values, by reverting.
-    ;; This doesn't work as expected.  Instead, they have their
-    ;; initial values `foo'.
     (should (equal custom--test-user-option 'baz))
     (should (equal custom--test-variable 'baz))))