From fa9326f7e77f85cab9627face82465ea1af261bd Mon Sep 17 00:00:00 2001 From: Mauro Aranda Date: Fri, 4 Sep 2020 11:46:33 -0300 Subject: [PATCH] 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 7581457ce8..cc445fe765 100644 --- a/lisp/custom.el +++ b/lisp/custom.el @@ -907,7 +907,15 @@ custom-push-theme (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 @@ enable-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 @@ disable-theme (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 766e484498..7853c84bb6 100644 --- a/test/lisp/custom-tests.el +++ b/test/lisp/custom-tests.el @@ -99,7 +99,6 @@ custom--test-variable ;; 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 @@ custom--test-theme-variables (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)))) -- 2.28.0