From: Mauro Aranda <maurooaranda@gmail.com>
To: 21355@debbugs.gnu.org
Cc: Greg Lucas <greg@glucas.net>
Subject: bug#21355: 24.4; Loading a theme causes session customizations to be saved
Date: Tue, 8 Sep 2020 09:32:30 -0300 [thread overview]
Message-ID: <CABczVwfB6WmyDjeEJ=0sW=f_AH-Lon5n+f9yPARrmdHuwZPcgg@mail.gmail.com> (raw)
In-Reply-To: <CABczVwcZn_K8yejDopKPkGUbM=91ZvtMAuyKnhjnXDtx7__=pQ@mail.gmail.com>
[-- 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
next prev parent reply other threads:[~2020-09-08 12:32 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2020-11-06 12:31 ` Mauro Aranda
2021-05-10 11:35 ` Lars Ingebrigtsen
2021-05-11 11:53 ` Mauro Aranda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CABczVwfB6WmyDjeEJ=0sW=f_AH-Lon5n+f9yPARrmdHuwZPcgg@mail.gmail.com' \
--to=maurooaranda@gmail.com \
--cc=21355@debbugs.gnu.org \
--cc=greg@glucas.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.