bug#34027: 27.0.50; disable-theme resets variables to their initial values

bug#34027: 27.0.50; disable-theme resets variables to their initial values

[Michael Albinus]

Scenario:

- Declare a variable (either defvar or defcustom)
- Change the initial value to something else
- Enable a theme, which changes the value to something different, again
- Disable the theme

I would expect that the variable has been reset to the value prior
enabling the theme. But it is reset to the initial value.

The documentation of disable-theme isn't helpful, it just says "Disable
all variable and face settings defined by THEME."

I've added the test case custom--test-theme-variables in
test/lisp/custom-resources/custom--test-theme.el for showing the effect.


In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.1)
 of 2018-12-27 built on detlef
Repository revision: 13dfe15ef4bc63ce66daffdc502a92aa9be44d92
Repository branch: master
System Description: Ubuntu 18.10

Recent messages:
Opening connection to imap.gmx.net...done
Opening connection to outlook.office365.com via tls...
Opening connection to outlook.office365.com...done
Reading active file via nnml...
Reading incoming mail from pop...
nnml: Reading incoming mail (no new mail)...done
Reading active file via nnml...done
Reading active file via nndraft...done
Checking new news...done
Auto-saving...done

Configured using:
 'configure --with-file-notification=gfile'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY GFILENOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT
LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8

Major mode: Group

Minor modes in effect:
  global-magit-file-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  diff-auto-refine-mode: t
  async-bytecomp-package-mode: t
  override-global-mode: t
  gnus-undo-mode: t
  erc-notify-mode: t
  erc-notifications-mode: t
  display-time-mode: t
  shell-dirtrack-mode: t
  icomplete-mode: t
  show-paren-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-editing-commands hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-editing-commands
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-repl hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-repl
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/inferior-slime hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/inferior-slime
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-tramp hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-tramp
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-scheme hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-scheme
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-media hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-media
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-presentation-streams hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-presentation-streams
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-trace-dialog hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-trace-dialog
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-autodoc hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-autodoc
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-scratch hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-scratch
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-fontifying-fu hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-fontifying-fu
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-listener-hooks hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-listener-hooks
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-enclosing-context hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-enclosing-context
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-asdf hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-asdf
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-indentation hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-indentation
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-sbcl-exts hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-sbcl-exts
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-macrostep hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-macrostep
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-hyperdoc hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-hyperdoc
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-cl-indent hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-cl-indent
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-fuzzy hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-fuzzy
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-mdot-fu hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-mdot-fu
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-references hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-references
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-fancy-inspector hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-fancy-inspector
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-fancy hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-fancy
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-parse hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-parse
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-fancy-trace hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-fancy-trace
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-c-p-c hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-c-p-c
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-quicklisp hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-quicklisp
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-compiler-notes-tree hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-compiler-notes-tree
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-clipboard hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-clipboard
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-highlight-edits hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-highlight-edits
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-presentations hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-presentations
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-banner hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-banner
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-xref-browser hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-xref-browser
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-mrepl hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-mrepl
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-snapshot hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-snapshot
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/bridge hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/bridge
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-sprof hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-sprof
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-typeout-frame hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-typeout-frame
/home/albinus/.emacs.d/elpa/slime-20190105.2006/contrib/slime-package-fu hides /home/albinus/.emacs.d/elpa/slime-20190105.2006/slime-package-fu
/home/albinus/src/elpa/packages/debbugs/debbugs-org hides /home/albinus/.emacs.d/elpa/debbugs-0.16/debbugs-org
/home/albinus/src/elpa/packages/debbugs/debbugs-gnu hides /home/albinus/.emacs.d/elpa/debbugs-0.16/debbugs-gnu
/home/albinus/src/elpa/packages/debbugs/debbugs hides /home/albinus/.emacs.d/elpa/debbugs-0.16/debbugs
/home/albinus/src/elpa/packages/debbugs/debbugs-autoloads hides /home/albinus/.emacs.d/elpa/debbugs-0.16/debbugs-autoloads
/home/albinus/src/elpa/packages/debbugs/debbugs-pkg hides /home/albinus/.emacs.d/elpa/debbugs-0.16/debbugs-pkg
/home/albinus/src/elpa/packages/debbugs/debbugs-browse hides /home/albinus/.emacs.d/elpa/debbugs-0.16/debbugs-browse
~/lisp/telepathy hides /home/albinus/.emacs.d/elpa/telepathy-20131209.1258/telepathy
/home/albinus/src/elpa/packages/tramp-theme/tramp-theme hides /home/albinus/.emacs.d/elpa/tramp-theme-0.2/tramp-theme
/home/albinus/src/elpa/packages/tramp-theme/tramp-theme-autoloads hides /home/albinus/.emacs.d/elpa/tramp-theme-0.2/tramp-theme-autoloads
/home/albinus/src/elpa/packages/tramp-theme/tramp-theme-pkg hides /home/albinus/.emacs.d/elpa/tramp-theme-0.2/tramp-theme-pkg
/home/albinus/src/tramp/lisp/tramp-smb hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-smb
/home/albinus/src/tramp/lisp/tramp-sudoedit hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-sudoedit
/home/albinus/src/tramp/lisp/tramp-uu hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-uu
/home/albinus/src/tramp/lisp/tramp-adb hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-adb
/home/albinus/src/tramp/lisp/tramp-compat hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-compat
/home/albinus/src/tramp/lisp/tramp-archive hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-archive
/home/albinus/src/tramp/lisp/tramp hides /usr/local/share/emacs/27.0.50/lisp/net/tramp
/home/albinus/src/tramp/lisp/trampver hides /usr/local/share/emacs/27.0.50/lisp/net/trampver
/home/albinus/src/tramp/lisp/tramp-rclone hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-rclone
/home/albinus/src/tramp/lisp/tramp-ftp hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-ftp
/home/albinus/src/tramp/lisp/tramp-cmds hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-cmds
/home/albinus/src/tramp/lisp/tramp-gvfs hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-gvfs
/home/albinus/src/tramp/lisp/tramp-loaddefs hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-loaddefs
~/lisp/dbus hides /usr/local/share/emacs/27.0.50/lisp/net/dbus
/home/albinus/src/tramp/lisp/tramp-sh hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-sh
/home/albinus/src/tramp/lisp/tramp-cache hides /usr/local/share/emacs/27.0.50/lisp/net/tramp-cache

Features:
(shadow emacsbug eww mm-url mailalias nnir vc-hg ediff-vers xdg swiper
docker-volume docker-network docker-machine docker-image
docker-container docker-process docker-utils docker-group tablist
tablist-filter semantic/wisent/comp semantic/wisent
semantic/wisent/wisent semantic/util-modes semantic/util semantic
semantic/tag semantic/lex semantic/fw cedet s ghub-graphql treepy
graphql ghub let-alist git-rebase ediff-merg ediff-wind ediff-diff
ediff-mult ediff-help ediff-init ediff-util ediff magit-bookmark
bookmark recentf magit-submodule magit-obsolete magit-blame magit-stash
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log magit-diff smerge-mode
magit-core magit-autorevert autorevert filenotify magit-process
magit-margin magit-mode git-commit magit-git magit-section magit-utils
magit-popup which-func vc-git diff-mode crm log-edit pcvs-util
with-editor async-bytecomp php-project mode-local imenu speedbar
sb-image ezimage dframe flymake-proc flymake add-log cc-langs cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs skeleton slime-repl-tests slime-indentation slime-tests
tree-widget ert ewoc debug backtrace inferior-slime slime-cl-indent
cl-indent slime-trace-dialog slime-fontifying-fu slime-package-fu
slime-references slime-compiler-notes-tree slime-scratch
slime-presentations bridge slime-macrostep macrostep slime-mdot-fu
slime-enclosing-context slime-fuzzy slime-fancy-trace
slime-fancy-inspector slime-c-p-c slime-autodoc slime-editing-commands
slime-repl elp slime-parse grep gud apropos cl slime arc-mode
archive-mode noutline outline hyperspec ivy use-package-ensure
use-package-delight use-package-diminish use-package-bind-key bind-key
use-package-core warnings esh-var esh-cmd esh-opt esh-io esh-ext
esh-proc esh-arg esh-groups eshell esh-module esh-mode esh-util server
dired-aux async tempo ispell etags multifile generator xref project
find-func company-dabbrev company-template company delsel colir
ivy-overlay ffap color hydra lv easy-mmode pcase compile autoload
radix-tree lisp-mnt tar-mode misearch multi-isearch tmm time-stamp
url-http url-gw url-auth url-queue url-cache url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util shr
text-property-search svg dom browse-url flow-fill mm-archive qp gnus-dup
sort gnus-cite mail-extr gnus-bcklg gnus-async cl-extra help-mode
gnus-ml pop3 utf-7 nndraft nnmh nnml gnutls network-stream nsm
gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-art
mm-uu mml2015 mm-view mml-smime smime dig mailcap gnus-cache gnus-sum
nnnil smtpmail sendmail gnus-demon nntp gnus-group gnus-undo gnus-start
gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int
gnus-range message rmc puny rfc822 mml mml-sec epa derived epg mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
gnus-win gnus nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045
ietf-drums time-date mail-utils mm-util mail-prsvr wid-edit term/xterm
xterm elec-pair erc-notify erc-networks erc-desktop-notifications
erc-match notifications dbus xml erc-goodies erc erc-backend erc-compat
thingatpt pp erc-loaddefs cperl-mode time tramp-sh kubernetes-tramp
lxc-tramp lxd-tramp subr-x docker-tramp tramp-cache vagrant-tramp dash
term disp-table ehelp tramp tramp-loaddefs trampver tramp-compat
ucs-normalize shell pcomplete comint ansi-color ring parse-time
format-spec advice ido jka-compr icomplete paren vc vc-dispatcher dired
dired-loaddefs mule-util edmacro kmacro rx slime-autoloads info
finder-inf package easymenu epg-config url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json map url-vars seq byte-opt gv bytecomp byte-compile
cconv cl-loaddefs cl-lib tooltip eldoc electric uniquify ediff-hook
vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core term/tty-colors frame cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded 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 threads dbusbind
gfilenotify lcms2 dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 1744851 334013)
 (symbols 48 68969 1)
 (strings 32 263536 29104)
 (string-bytes 1 8414120)
 (vectors 16 107386)
 (vector-slots 8 2266284 149460)
 (floats 8 633 1199)
 (intervals 56 174943 14326)
 (buffers 992 63))

bug#34027: 27.0.50; disable-theme resets variables to their initial values

[Glenn Morris]

Duplicate of bug#15687?

bug#34027: 27.0.50; disable-theme resets variables to their initial values

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

Hi Glenn.

> Duplicate of bug#15687?

Yes. I'll merge both bugs, hopefully something will happen then.

(And maybe I'll become impatient, and try to solve it. My report was
triggered by the ELPA package tramp-theme.el, which suffers from this
problem.)

Best regards, Michael.

bug#34027: 27.0.50; disable-theme resets variables to their initial values

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

Michael Albinus <michael.albinus@gmx.de> writes:

> I've added the test case custom--test-theme-variables in
> test/lisp/custom-resources/custom--test-theme.el for showing the effect.

The test is failing for a different reason.  With the call to
custom-set-variables, both variables will end up having a user theme
setting, and since the user theme has the highest priority, enabling
the custom--test theme makes no difference.  The following check fails,
because of the user theme having the highest priority:

;; The variables have the theme values.
(should (equal custom--test-user-option 'bar))
(should (equal custom--test-variable 'bar))

IOW, the variables have the user theme values, and not the custom--test
theme values.

Best regards,
Mauro.

[-- Attachment #2: Type: text/html, Size: 946 bytes --]

bug#34027: 27.0.50; disable-theme resets variables to their initial values

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 1153 bytes --]

Michael Albinus <michael.albinus@gmx.de> writes:

> Scenario:
>
> - Declare a variable (either defvar or defcustom)
> - Change the initial value to something else
> - Enable a theme, which changes the value to something different, again
> - Disable the theme
>
> I would expect that the variable has been reset to the value prior
> enabling the theme. But it is reset to the initial value.
>

There is code in custom-push-theme that attempts to handle this case,
but it was being skipped because custom didn't think it should apply the
settings of the theme (and at the same time, preserve priors
customizations).

That is controlled by the variable custom--inhibit-theme-enable, and
we should bind it to nil in enable-theme, because we are definitely
enabling it.  Once we do that, it is just a matter of using
custom-push-theme to handle the case like it's supposed to.

My patch does that, and introduces an extra check in custom-push-theme,
because while testing I found another instance of Bug#28904.  The rest
this patch does is changing the test because of the comments I made in
another post to this bug, and we now can expect the test to pass.

[-- Attachment #1.2: Type: text/html, Size: 1343 bytes --]

[-- Attachment #2: 0001-Preserve-user-customizations-after-disabling-a-theme.patch --]
[-- Type: text/x-patch, Size: 4764 bytes --]

From fa9326f7e77f85cab9627face82465ea1af261bd Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
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

bug#34027: 27.0.50; disable-theme resets variables to their initial values

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> That is controlled by the variable custom--inhibit-theme-enable, and
> we should bind it to nil in enable-theme, because we are definitely
> enabling it.  Once we do that, it is just a matter of using
> custom-push-theme to handle the case like it's supposed to.
>
> My patch does that, and introduces an extra check in custom-push-theme,
> because while testing I found another instance of Bug#28904.  The rest
> this patch does is changing the test because of the comments I made in
> another post to this bug, and we now can expect the test to pass.

Thanks, looks good to me (although I'm not all that familiar with the
custom/theme integration), so I've applied your patch to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no