* bug#21355: 24.4; Loading a theme causes session customizations to be saved
@ 2015-08-26 21:04 Greg Lucas
2020-09-07 19:50 ` Mauro Aranda
0 siblings, 1 reply; 6+ messages in thread
From: Greg Lucas @ 2015-08-26 21:04 UTC (permalink / raw)
To: 21355
Loading a theme has the unexpected side of effect of making
customizations in the current session saved to the custom-file the next
time it is updated.
I reproduced this problem by starting Emacs with an new empty user home
directory (since using -q would prevent saving customizations at all).
Then:
M-x customize-variable sentence-end-double-space -- toggle the value,
and Set for Current Session.
M-x load-theme deeper-blue
M-x customize-variable user-full-name -- set a value and Save for Future
Sessions.
I would expect only the user-full-name to be saved, but the resulting
.emacs file contains:
(custom-set-variables
;; custom-set-variables was added by Custom.
;; If you edit it by hand, you could mess it up, so be careful.
;; Your init file should contain only one such instance.
;; If there is more than one, they won't work right.
'(sentence-end-double-space nil)
'(user-full-name "Greg Lucas"))
(custom-set-faces
;; custom-set-faces was added by Custom.
;; If you edit it by hand, you could mess it up, so be careful.
;; Your init file should contain only one such instance.
;; If there is more than one, they won't work right.
)
From testing this I've found that at the time I call `load-theme` all
customizations made for the current session will get marked to be saved.
Changes made after load-theme behave as expected and do not get saved
unless I explicitly choose to save them.
In GNU Emacs 24.4.1 (i686-pc-mingw32)
of 2014-10-20 on LEG570
Windowing system distributor `Microsoft Corp.', version 6.3.9600
Important settings:
value of $LANG: ENU
locale-coding-system: cp1252
Major mode: Emacs-Lisp
Minor modes in effect:
tooltip-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
blink-cursor-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
line-number-mode: t
transient-mark-mode: t
Recent input:
<lwindow> M-x c u s t o m i z e - v a r <tab> <return>
s h o w - p a <tab> <tab> <tab> <C-backspace> - <backspace>
<tab> <tab> <C-backspace> <C-backspace> <C-backspace>
<tab> C-g C-g M-x c u s t - v <tab> a r <tab> <return>
s e n t e n <tab> <tab> - d <tab> <return> <help-echo>
<down-mouse-1> <mouse-1> <help-echo> <down-mouse-1>
<help-echo> <help-echo> M-x l o a d - t h e m e <return>
d e e <tab> <return> M-x c u s t o m i z e - v <tab>
<return> s h o w - p a <tab> <C-backspace> <C-backspace>
<C-backspace> u s e r <tab> f <tab> <return> <down-mouse-1>
<mouse-1> G r e g SPC L u c a s <help-echo> <help-echo>
<down-mouse-1> C-x C-f ~ / . e m <tab> <return> M-x
e m a c s - r e <tab> <C-backspace> <C-backspace> r
e p o <tab> r <tab> <return>
Recent messages:
Creating customization setup...done
To install your edits, invoke [State] and choose the Set operation
Creating customization items...
Creating customization items ...done
Resetting customization items...done
Creating customization setup...done
(New file)
Saving file c:/temp/emacs-home/.emacs...
Wrote c:/temp/emacs-home/.emacs [2 times]
Making completion list...
Load-path shadows:
None found.
Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils deeper-blue-theme help-mode help-fns cus-edit
easymenu cus-start cus-load wid-edit cl-loaddefs cl-lib time-date
tooltip electric uniquify ediff-hook vc-hooks lisp-float-type mwheel
dos-w32 ls-lisp w32-common-fns disp-table w32-win w32-vars tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
w32notify w32 multi-tty emacs)
Memory information:
((conses 8 99251 6599)
(symbols 32 19304 0)
(miscs 32 270 184)
(strings 16 15490 4166)
(string-bytes 1 403422)
(vectors 8 10810)
(vector-slots 4 394593 5356)
(floats 8 68 253)
(intervals 28 353 19)
(buffers 508 17))
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#21355: 24.4; Loading a theme causes session customizations to be saved
2015-08-26 21:04 bug#21355: 24.4; Loading a theme causes session customizations to be saved Greg Lucas
@ 2020-09-07 19:50 ` Mauro Aranda
2020-09-08 12:32 ` Mauro Aranda
0 siblings, 1 reply; 6+ messages in thread
From: Mauro Aranda @ 2020-09-07 19:50 UTC (permalink / raw)
To: 21355; +Cc: Greg Lucas
[-- Attachment #1: Type: text/plain, Size: 4711 bytes --]
Greg Lucas <greg@glucas.net> writes:
> Loading a theme has the unexpected side of effect of making
> customizations in the current session saved to the custom-file the next
> time it is updated.
>
> I reproduced this problem by starting Emacs with an new empty user home
> directory (since using -q would prevent saving customizations at all).
>
> Then:
>
> M-x customize-variable sentence-end-double-space -- toggle the value,
> and Set for Current Session.
> M-x load-theme deeper-blue
> M-x customize-variable user-full-name -- set a value and Save for Future
> Sessions.
>
> I would expect only the user-full-name to be saved, but the resulting
> .emacs file contains:
>
> (custom-set-variables
> ;; custom-set-variables was added by Custom.
> ;; If you edit it by hand, you could mess it up, so be careful.
> ;; Your init file should contain only one such instance.
> ;; If there is more than one, they won't work right.
> '(sentence-end-double-space nil)
> '(user-full-name "Greg Lucas"))
> (custom-set-faces
> ;; custom-set-faces was added by Custom.
> ;; If you edit it by hand, you could mess it up, so be careful.
> ;; Your init file should contain only one such instance.
> ;; If there is more than one, they won't work right.
> )
>
>
>
> From testing this I've found that at the time I call `load-theme` all
> customizations made for the current session will get marked to be saved.
> Changes made after load-theme behave as expected and do not get saved
> unless I explicitly choose to save them.
This one is really tricky.
When saving the variables to the custom-file with
custom-save-variables, we are interested in the symbols that have a
non-nil saved-value property and that don't have any theme applied:
(get symbol 'theme-value) ==> nil
or have the user theme preference applied:
(caar (get symbol 'theme-value)) ==> 'user
So we shouldn't let those combinations happen by accident.
The offending code is in custom-theme-recalc-variable: when there is a
custom theme that specifies a value for the variable, it puts that into
the 'saved-value property. And custom-theme-recalc-variable is called
when enabling or disabling a theme. So what happens in the recipe is:
- The user customizes some variable, so now:
(get VARIABLE 'theme-value) ==> ((user SETTING))
(get VARIABLE 'saved-value) ==> nil
- Then the user loads a theme with M-x load-theme. So we enable it, and
then we give the priority to the user, so we re-enable the user theme.
When we enable the user theme, we end up with:
(get VARIABLE 'theme-value) ==> ((user SETTING))
(get VARIABLE 'saved-value) ==> (SETTING)
- If later in the session `custom-save-all' runs, then we save that
setting by mistake.
The code that depends on that side-effect of
custom-theme-recalc-variable is the custom-initialize-* functions (at
least in custom.el). Those functions need to know if there is one value
stashed for the variable, to set it accordingly when initializing it.
And we stash the value when we load the custom-file or when loading a
theme. So we are using the saved-value property for stashing this
value, which seems unfortunate to me.
Furthermore, there are some other scenarios where the bug happens:
1. Suppose we load a theme that has a setting for a bound variable.
Then we have:
(get VARIABLE 'theme-value) ==> ((THEME THEME-SETTING))
(get VARIABLE 'saved-value) ==> (THEME-SETTING)
Since THEME is not the user theme, we are not saving it in
`custom-save-all'. But, when we disable the theme we have:
(get VARIABLE 'theme-value) ==> nil
(get VARIABLE 'saved-value) ==> (THEME-SETTING)
So if later custom-save-all runs, we lose again.
2. Now say we load a theme that has a setting for a variable that is
initially void in our session.
(get VARIABLE 'theme-value) ==> ((THEME THEME-SETTING))
(get VARIABLE 'saved-value) ==> (THEME-SETTING)
And then Emacs finds the option. Then:
VARIABLE ==> THEME-SETTING
(get VARIABLE 'theme-value) ==> ((THEME THEME-SETTING))
(get VARIABLE 'saved-value) ==> (THEME-SETTING)
So, that shows that stashing the value worked. But say we customize the
variable for the session:
VARIABLE ==> OUR-SETTING
(get VARIABLE 'theme-value) ==> ((user OUR-SETTING) (THEME THEME-SETTING))
(get VARIABLE 'saved-value) ==> (THEME-SETTING)
And we lose again.
I think that if we want to keep stashing the value under the
saved-value property, we could try to stash the value only if we need
to. That is, if default-toplevel-value errors out, which would mean
custom-initialize-* will need to know if there is a saved-value. But
then we have to make some tricks to reset the saved-value if we need
to.
I hope this analysis is helpful, I tried to keep it as short as I
could.
[-- Attachment #2: Type: text/html, Size: 5440 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#21355: 24.4; Loading a theme causes session customizations to be saved
2020-09-07 19:50 ` Mauro Aranda
@ 2020-09-08 12:32 ` Mauro Aranda
2020-11-06 12:31 ` Mauro Aranda
0 siblings, 1 reply; 6+ messages in thread
From: Mauro Aranda @ 2020-09-08 12:32 UTC (permalink / raw)
To: 21355; +Cc: Greg Lucas
[-- Attachment #1.1: Type: text/plain, Size: 60 bytes --]
I attach a patch with three tests that demonstrate the bug.
[-- Attachment #1.2: Type: text/html, Size: 107 bytes --]
[-- Attachment #2: 0001-Add-tests-demonstrating-Bug-21355.patch --]
[-- Type: text/x-patch, Size: 7614 bytes --]
From cd0266909efd6c2f6763ef9aff18f26149651a8a Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Tue, 8 Sep 2020 09:23:40 -0300
Subject: [PATCH] Add tests demonstrating Bug#21355
* test/lisp/custom-resources/custom--test-saved-value-theme.el: A new
theme.
* test/lisp/custom-tests.el
(custom-test-no-saved-value-after-enabling-theme)
(custom-test-no-saved-value-after-enabling-theme-2)
(custom-test-no-saved-value-after-customizing-option): New tests.
---
.../custom--test-saved-value-theme.el | 11 ++
test/lisp/custom-tests.el | 111 ++++++++++++++++++
2 files changed, 122 insertions(+)
create mode 100644 test/lisp/custom-resources/custom--test-saved-value-theme.el
diff --git a/test/lisp/custom-resources/custom--test-saved-value-theme.el b/test/lisp/custom-resources/custom--test-saved-value-theme.el
new file mode 100644
index 0000000000..eea1547916
--- /dev/null
+++ b/test/lisp/custom-resources/custom--test-saved-value-theme.el
@@ -0,0 +1,11 @@
+;;; custom--test-saved-value-theme.el -- A test theme to test Bug#21355. -*- lexical-binding:t -*-
+
+(deftheme custom--test-saved-value
+ "A test theme to test against saving session customizations by mistake.")
+
+(custom-theme-set-variables
+ 'custom--test-saved-value
+ '(custom--test-bug-21355-before 'before)
+ '(custom--test-bug-21355-after 'after))
+
+(provide-theme 'custom--test-saved-value)
diff --git a/test/lisp/custom-tests.el b/test/lisp/custom-tests.el
index cabbf861f1..9cf25be87a 100644
--- a/test/lisp/custom-tests.el
+++ b/test/lisp/custom-tests.el
@@ -147,4 +147,115 @@ custom-test-show-comment-preserves-changes
(widget-apply field :value-to-internal origvalue)
"bar"))))))
+;; The following three tests demonstrate Bug#21355.
+;; In this one, we set an user option for the current session,
+;; then we enable a theme that doesn't have a setting for it, and we end up
+;; with a 'saved-value property, and the caar of the 'theme-value property
+;; is 'user, which means we would end up saving a setting that was meant
+;; for the current session only.
+(ert-deftest custom-test-no-saved-value-after-enabling-theme ()
+ "Test that we don't record a saved-value property when we shouldn't."
+ :expected-result :failed
+ (let ((custom-theme-load-path
+ `(,(expand-file-name
+ "custom-resources"
+ (expand-file-name "lisp" (getenv "EMACS_TEST_DIRECTORY"))))))
+ (customize-option 'mark-ring-max)
+ (let* ((field (seq-find (lambda (widget)
+ (eq mark-ring-max (widget-value widget)))
+ widget-field-list))
+ (parent (widget-get field :parent)))
+ ;; We move to the editable widget, modify the value and save it.
+ (goto-char (widget-field-text-end field))
+ (insert "0")
+ (widget-apply parent :custom-set)
+ ;; Just setting for the current session should not store a 'saved-value
+ ;; property.
+ (should-not (get 'mark-ring-max 'saved-value))
+ ;; Now we enable and disable our test theme.
+ (load-theme 'custom--test 'no-confirm)
+ (disable-theme 'custom--test)
+ ;; Since the user customized the option, this happens.
+ (should (eq (caar (get 'mark-ring-max 'theme-value)) 'user))
+ ;; But this should not happen: the 'saved-value property should still
+ ;; be nil.
+ (should-not (get 'mark-ring-max 'saved-value)))))
+
+;; In this second test, we load a theme that has a setting for the user option
+;; above. Regardless, the same happens: we end up with a non-nil saved-value
+;; property, and a user setting active in the 'theme-value property, which
+;; means we could inadvertently save the session setting in the custom-file.
+(defcustom custom--test-bug-21355-before 'foo
+ "User option for `custom-test-no-saved-value-after-enabling-theme-2'."
+ :type 'symbol :group 'emacs)
+
+(ert-deftest custom-test-no-saved-value-after-enabling-theme-2 ()
+ "Test that we don't record a saved-value property when we shouldn't."
+ :expected-result :failed
+ (let ((custom-theme-load-path
+ `(,(file-name-as-directory (expand-file-name
+ "custom-resources"
+ (expand-file-name "lisp" (getenv "EMACS_TEST_DIRECTORY")))))))
+ (customize-option 'custom--test-bug-21355-before)
+ (let* ((field (seq-find (lambda (widget)
+ (eq custom--test-bug-21355-before (widget-value widget)))
+ widget-field-list))
+ (parent (widget-get field :parent)))
+ ;; We move to the editable widget, modify the value and save it.
+ (goto-char (widget-field-text-end field))
+ ;; Modify the value and save it.
+ (insert "bar")
+ (widget-apply parent :custom-set)
+ ;; Just setting for the current session should not store a 'saved-value
+ ;; property.
+ (should-not (get 'custom--test-bug-21355-before 'saved-value))
+ ;; Now we load our test theme, which has a setting for
+ ;; `custom--test-user-option'.
+ (load-theme 'custom--test-saved-value 'no-confirm)
+ (enable-theme 'custom--test-saved-value)
+ ;; We still shouldn't have a non-nil 'saved-value property, even if
+ ;; the theme has a setting for the variable.
+ ;; Since the user customized the option, this happens.
+ (should (eq (caar (get 'custom--test-bug-21355-before 'theme-value))
+ 'user))
+ ;; But this should not happen.
+ (should-not (get 'custom--test-bug-21355-before 'saved-value)))))
+
+;; And in this test we check that stashing a theme value for a not yet defined
+;; option works, but that later on if the user customizes the option for the
+;; current session, we might save the theme setting in the custom file.
+(ert-deftest custom-test-no-saved-value-after-customizing-option ()
+ "Test that we don't have a non-nil saved-value after customizing an option."
+ :expected-result :failed
+ (let ((custom-theme-load-path
+ `(,(file-name-as-directory (expand-file-name
+ "custom-resources"
+ (expand-file-name "lisp" (getenv "EMACS_TEST_DIRECTORY")))))))
+ ;; Check that we correctly stashed the value.
+ (should (and (not (boundp 'custom--test-bug-21355-after))
+ (eq (eval
+ (car (get 'custom--test-bug-21355-after 'saved-value)))
+ 'after)))
+ ;; Now Emacs finds the defcustom.
+ (defcustom custom--test-bug-21355-after 'initially "..."
+ :type 'symbol :group 'emacs)
+ ;; And we used the stashed value correctly.
+ (should (and (boundp 'custom--test-bug-21355-after)
+ (eq custom--test-bug-21355-after 'after)))
+ ;; Now we customize it.
+ (customize-option 'custom--test-bug-21355-after)
+ (let* ((field (seq-find (lambda (widget)
+ (eq custom--test-bug-21355-after (widget-value widget)))
+ widget-field-list))
+ (parent (widget-get field :parent)))
+ ;; We move to the editable widget, modify the value and save it.
+ (goto-char (widget-field-text-end field))
+ (insert "bar")
+ (widget-apply parent :custom-set)
+ ;; The user customized the variable, so this should happen.
+ (should (eq (caar (get 'custom--test-bug-21355-after 'theme-value))
+ 'user))
+ ;; But it was only for the current session, so this should not happen.
+ (should-not (get 'custom--test-bug-21355-after 'saved-value)))))
+
;;; custom-tests.el ends here
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#21355: 24.4; Loading a theme causes session customizations to be saved
2020-09-08 12:32 ` Mauro Aranda
@ 2020-11-06 12:31 ` Mauro Aranda
2021-05-10 11:35 ` Lars Ingebrigtsen
0 siblings, 1 reply; 6+ messages in thread
From: Mauro Aranda @ 2020-11-06 12:31 UTC (permalink / raw)
To: 21355; +Cc: Greg Lucas
[-- Attachment #1.1: Type: text/plain, Size: 178 bytes --]
Mauro Aranda <maurooaranda@gmail.com> writes:
> I attach a patch with three tests that demonstrate the bug.
And now I attach a patch to fix this, with the three tests updated.
[-- Attachment #1.2: Type: text/html, Size: 269 bytes --]
[-- Attachment #2: 0001-Avoid-saving-session-customizations-in-the-custom-fi.patch --]
[-- Type: text/x-patch, Size: 10848 bytes --]
From 24a895f54d2dc6333c34cca69e600c494e697a26 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Fri, 6 Nov 2020 08:59:32 -0300
Subject: [PATCH] Avoid saving session customizations in the custom-file
* lisp/custom.el (custom-theme-recalc-variable): Only stash theme
settings for void variables.
(custom-declare-variable): After initializing a variable, unstash a
theme setting, if present.
(disable-theme): When disabling a theme, maybe unstash a theme
setting.
* test/lisp/custom-resources/custom--test-theme.el: Add two settings
for testing the fix.
* test/lisp/custom--tests.el (custom--test-bug-21355-before): New user
option for testing.
(custom-test-no-saved-value-after-enabling-theme)
(custom-test-no-saved-value-after-enabling-theme-2)
(custom-test-no-saved-value-after-customizing-option): New tests.
---
lisp/custom.el | 40 ++++++-
.../custom-resources/custom--test-theme.el | 4 +-
test/lisp/custom-tests.el | 104 ++++++++++++++++++
3 files changed, 142 insertions(+), 6 deletions(-)
diff --git a/lisp/custom.el b/lisp/custom.el
index cee4589543..0b23726361 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -205,7 +205,22 @@ custom-declare-variable
(put symbol 'custom-requests requests)
;; Do the actual initialization.
(unless custom-dont-initialize
- (funcall initialize symbol default)))
+ (funcall initialize symbol default)
+ ;; If there is a value under saved-value that wasn't saved by the user,
+ ;; reset it: we used that property to stash the value, but we don't need
+ ;; it anymore.
+ ;; This can happen given the following:
+ ;; 1. The user loaded a theme that had a setting for an unbound
+ ;; variable, so we stashed the theme setting under the saved-value
+ ;; property in `custom-theme-recalc-variable'.
+ ;; 2. Then, Emacs evaluated the defcustom for the option
+ ;; (e.g., something required the file where the option is defined).
+ ;; If we don't reset it and the user later sets this variable via
+ ;; Customize, we might end up saving the theme setting in the custom-file.
+ ;; See the test `custom-test-no-saved-value-after-customizing-option'.
+ (let ((theme (caar (get symbol 'theme-value))))
+ (when (and theme (not (eq theme 'user)) (get symbol 'saved-value))
+ (put symbol 'saved-value nil)))))
(run-hooks 'custom-define-hook)
symbol)
@@ -1458,7 +1473,15 @@ disable-theme
(custom-push-theme prop symbol theme 'reset)
(cond
((eq prop 'theme-value)
- (custom-theme-recalc-variable symbol))
+ (custom-theme-recalc-variable symbol)
+ ;; We might have to reset the stashed value of the variable, if
+ ;; no other theme is customizing it. Without this, loading a theme
+ ;; that has a setting for an unbound user option and then disabling
+ ;; it will leave this lingering setting for the option, and if then
+ ;; Emacs evaluates the defcustom the saved-value might be used to
+ ;; set the variable. (Bug#20766)
+ (unless (get symbol 'theme-value)
+ (put symbol 'saved-value nil)))
((eq prop 'theme-face)
;; If the face spec specified by this theme is in the
;; saved-face property, reset that property.
@@ -1507,9 +1530,16 @@ custom-variable-theme-value
(defun custom-theme-recalc-variable (variable)
"Set VARIABLE according to currently enabled custom themes."
(let ((valspec (custom-variable-theme-value variable)))
- (if valspec
- (put variable 'saved-value valspec)
- (setq valspec (get variable 'standard-value)))
+ ;; We used to save VALSPEC under the saved-value property unconditionally,
+ ;; but that is a recipe for trouble because we might end up saving session
+ ;; customizations if the user loads a theme. (Bug#21355)
+ ;; It's better to only use the saved-value property to stash the value only
+ ;; if we really need to stash it (i.e., VARIABLE is void).
+ (condition-case nil
+ (default-toplevel-value variable) ; See if it doesn't fail.
+ (void-variable (when valspec
+ (put variable 'saved-value valspec))))
+ (or valspec (setq valspec (get variable 'standard-value)))
(if (and valspec
(or (get variable 'force-value)
(default-boundp variable)))
diff --git a/test/lisp/custom-resources/custom--test-theme.el b/test/lisp/custom-resources/custom--test-theme.el
index 4ced98a50b..6b1cca3c15 100644
--- a/test/lisp/custom-resources/custom--test-theme.el
+++ b/test/lisp/custom-resources/custom--test-theme.el
@@ -6,6 +6,8 @@ custom--test
(custom-theme-set-variables
'custom--test
'(custom--test-user-option 'bar)
- '(custom--test-variable 'bar))
+ '(custom--test-variable 'bar)
+ '(custom--test-bug-21355-before 'before)
+ '(custom--test-bug-21355-after 'after))
(provide-theme 'custom--test)
diff --git a/test/lisp/custom-tests.el b/test/lisp/custom-tests.el
index a1451cf0ce..15661fb78f 100644
--- a/test/lisp/custom-tests.el
+++ b/test/lisp/custom-tests.el
@@ -156,4 +156,108 @@ check-for-wrong-custom-types
(load custom-test-admin-cus-test)
(should (null (cus-test-opts t))))
+;; The following three tests demonstrate Bug#21355.
+;; In this one, we set an user option for the current session and then
+;; we enable a theme that doesn't have a setting for it, ending up with
+;; a non-nil saved-value property. Since the `caar' of the theme-value
+;; property is user (i.e., the user theme setting is active), we might
+;; save the setting to the custom-file, even though it was meant for the
+;; current session only. So there should be a nil saved-value property
+;; for this test to pass.
+(ert-deftest custom-test-no-saved-value-after-enabling-theme ()
+ "Test that we don't record a saved-value property when we shouldn't."
+ (let ((custom-theme-load-path `(,(ert-resource-directory))))
+ (customize-option 'mark-ring-max)
+ (let* ((field (seq-find (lambda (widget)
+ (eq mark-ring-max (widget-value widget)))
+ widget-field-list))
+ (parent (widget-get field :parent)))
+ ;; Move to the editable widget, modify the value and save it.
+ (goto-char (widget-field-text-end field))
+ (insert "0")
+ (widget-apply parent :custom-set)
+ ;; Just setting for the current session should not store a saved-value
+ ;; property.
+ (should-not (get 'mark-ring-max 'saved-value))
+ ;; Now enable and disable the test theme.
+ (load-theme 'custom--test 'no-confirm)
+ (disable-theme 'custom--test)
+ ;; Since the user customized the option, this is OK.
+ (should (eq (caar (get 'mark-ring-max 'theme-value)) 'user))
+ ;; The saved-value property should still be nil.
+ (should-not (get 'mark-ring-max 'saved-value)))))
+
+;; In this second test, we load a theme that has a setting for the user option
+;; above. We must check that we don't end up with a non-nil saved-value
+;; property and a user setting active in the theme-value property, which
+;; means we might inadvertently save the session setting in the custom-file.
+(defcustom custom--test-bug-21355-before 'foo
+ "User option for `custom-test-no-saved-value-after-enabling-theme-2'."
+ :type 'symbol :group 'emacs)
+
+(ert-deftest custom-test-no-saved-value-after-enabling-theme-2 ()
+ "Test that we don't record a saved-value property when we shouldn't."
+ (let ((custom-theme-load-path `(,(ert-resource-directory))))
+ (customize-option 'custom--test-bug-21355-before)
+ (let* ((field (seq-find
+ (lambda (widget)
+ (eq custom--test-bug-21355-before (widget-value widget)))
+ widget-field-list))
+ (parent (widget-get field :parent)))
+ ;; Move to the editable widget, modify the value and save it.
+ (goto-char (widget-field-text-end field))
+ (insert "bar")
+ (widget-apply parent :custom-set)
+ ;; Just setting for the current session should not store a saved-value
+ ;; property.
+ (should-not (get 'custom--test-bug-21355-before 'saved-value))
+ ;; Now load our test theme, which has a setting for
+ ;; `custom--test-bug-21355-before'.
+ (load-theme 'custom--test 'no-confirm 'no-enable)
+ (enable-theme 'custom--test)
+ ;; Since the user customized the option, this is OK.
+ (should (eq (caar (get 'custom--test-bug-21355-before 'theme-value))
+ 'user))
+ ;; But the saved-value property has to be nil, since the user didn't mark
+ ;; this variable to save for future sessions.
+ (should-not (get 'custom--test-bug-21355-before 'saved-value)))))
+
+(defvar custom--test-bug-21355-after)
+
+;; In this test, we check that stashing a theme value for a not yet defined
+;; option works, but that later on if the user customizes the option for the
+;; current session, we might save the theme setting in the custom file.
+(ert-deftest custom-test-no-saved-value-after-customizing-option ()
+ "Test for a nil saved-value after setting an option for the current session."
+ (let ((custom-theme-load-path `(,(ert-resource-directory))))
+ ;; Check that we correctly stashed the value.
+ (load-theme 'custom--test 'no-confirm 'no-enable)
+ (enable-theme 'custom--test)
+ (should (and (not (boundp 'custom--test-bug-21355-after))
+ (eq (eval
+ (car (get 'custom--test-bug-21355-after 'saved-value)))
+ 'after)))
+ ;; Now Emacs finds the defcustom.
+ (defcustom custom--test-bug-21355-after 'initially "..."
+ :type 'symbol :group 'emacs)
+ ;; And we used the stashed value correctly.
+ (should (and (boundp 'custom--test-bug-21355-after)
+ (eq custom--test-bug-21355-after 'after)))
+ ;; Now customize it.
+ (customize-option 'custom--test-bug-21355-after)
+ (let* ((field (seq-find (lambda (widget)
+ (eq custom--test-bug-21355-after
+ (widget-value widget)))
+ widget-field-list))
+ (parent (widget-get field :parent)))
+ ;; Move to the editable widget, modify the value and save it.
+ (goto-char (widget-field-text-end field))
+ (insert "bar")
+ (widget-apply parent :custom-set)
+ ;; The user customized the variable, so this is OK.
+ (should (eq (caar (get 'custom--test-bug-21355-after 'theme-value))
+ 'user))
+ ;; But it was only for the current session, so this should not happen.
+ (should-not (get 'custom--test-bug-21355-after 'saved-value)))))
+
;;; custom-tests.el ends here
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#21355: 24.4; Loading a theme causes session customizations to be saved
2020-11-06 12:31 ` Mauro Aranda
@ 2021-05-10 11:35 ` Lars Ingebrigtsen
2021-05-11 11:53 ` Mauro Aranda
0 siblings, 1 reply; 6+ messages in thread
From: Lars Ingebrigtsen @ 2021-05-10 11:35 UTC (permalink / raw)
To: Mauro Aranda; +Cc: Greg Lucas, 21355
Mauro Aranda <maurooaranda@gmail.com> writes:
> And now I attach a patch to fix this, with the three tests updated.
If I'm reading that patch correctly, it looks good to me, so I've now
applied it to Emacs 28. It didn't apply cleanly, so I fixed the patch
up manually, and hopefully that didn't lead to any merge errors.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#21355: 24.4; Loading a theme causes session customizations to be saved
2021-05-10 11:35 ` Lars Ingebrigtsen
@ 2021-05-11 11:53 ` Mauro Aranda
0 siblings, 0 replies; 6+ messages in thread
From: Mauro Aranda @ 2021-05-11 11:53 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: greg, 21355
Lars Ingebrigtsen <larsi@gnus.org> writes:
> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> And now I attach a patch to fix this, with the three tests updated.
>
> If I'm reading that patch correctly, it looks good to me, so I've now
> applied it to Emacs 28. It didn't apply cleanly, so I fixed the patch
> up manually, and hopefully that didn't lead to any merge errors.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-11 11:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-26 21:04 bug#21355: 24.4; Loading a theme causes session customizations to be saved Greg Lucas
2020-09-07 19:50 ` Mauro Aranda
2020-09-08 12:32 ` Mauro Aranda
2020-11-06 12:31 ` Mauro Aranda
2021-05-10 11:35 ` Lars Ingebrigtsen
2021-05-11 11:53 ` Mauro Aranda
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).