master 31c3f53: Preserve user customizations after disabling a theme

[Lars Ingebrigtsen]

branch: master
commit 31c3f53816feb4c40c52cf2840c171a9d631de66
Author: Mauro Aranda <maurooaranda@gmail.com>
Commit: Lars Ingebrigtsen <larsi@gnus.org>

    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).
---
 lisp/custom.el            | 21 +++++++++++++++------
 test/lisp/custom-tests.el |  8 +-------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/lisp/custom.el b/lisp/custom.el
index 7581457..cc445fe 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 766e484..7853c84 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))))