From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Mauro Aranda Newsgroups: gmane.emacs.bugs Subject: bug#21355: 24.4; Loading a theme causes session customizations to be saved Date: Mon, 7 Sep 2020 16:50:39 -0300 Message-ID: References: <1440623057.3987499.366976033.36C3C135@webmail.messagingengine.com> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="00000000000023f78c05aebe89cb" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="32643"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Greg Lucas To: 21355@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Sep 07 21:52:13 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kFNBV-0008OT-ID for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 07 Sep 2020 21:52:13 +0200 Original-Received: from localhost ([::1]:37750 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kFNBU-00010t-I4 for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 07 Sep 2020 15:52:12 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:59258) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kFNBK-0000xP-Vc for bug-gnu-emacs@gnu.org; Mon, 07 Sep 2020 15:52:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:39395) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kFNBK-0006l5-JP for bug-gnu-emacs@gnu.org; Mon, 07 Sep 2020 15:52:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kFNBK-0006D8-IA for bug-gnu-emacs@gnu.org; Mon, 07 Sep 2020 15:52:02 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: <1440623057.3987499.366976033.36C3C135@webmail.messagingengine.com> Resent-From: Mauro Aranda Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 07 Sep 2020 19:52:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 21355 X-GNU-PR-Package: emacs Original-Received: via spool by 21355-submit@debbugs.gnu.org id=B21355.159950826423805 (code B ref 21355); Mon, 07 Sep 2020 19:52:02 +0000 Original-Received: (at 21355) by debbugs.gnu.org; 7 Sep 2020 19:51:04 +0000 Original-Received: from localhost ([127.0.0.1]:50941 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kFNAN-0006Bs-Nb for submit@debbugs.gnu.org; Mon, 07 Sep 2020 15:51:04 -0400 Original-Received: from mail-wr1-f44.google.com ([209.85.221.44]:42311) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kFNAH-0006BD-TI for 21355@debbugs.gnu.org; Mon, 07 Sep 2020 15:51:02 -0400 Original-Received: by mail-wr1-f44.google.com with SMTP id c18so16813990wrm.9 for <21355@debbugs.gnu.org>; Mon, 07 Sep 2020 12:50:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to:cc; bh=kRFFQCAzPSqug/s9TuSfbcrQXz9M9tI/wJEAGRzzT/8=; b=Ntc+etVyBKbUtyAbWrH6LEm3I0gBTTyqArDT2e7MdoKA0/E6guihK1K3oHO6XMjkzK LvCA2DIVFZtyWrluGVBf+kUdot9uchM0Axu8IquM0e/yV7LbMcSwX7HbVTbSZPso0uDt s/RVaHiFPy4H5fL38i3iPKnVbMdgL6vv4lOH6Fashd7puSKzERdQVXGZErZo5aZ1PzeZ RTFQYF4FCrEb2FQ2pEsLA1S2YPCZ692OAIXcWJ1o16IUtCW+CmL8yil/Uu9/BGDBNf0b jW/Nd92h+aQPnL6Zef6rmA+3BuY6RNULmqdiO/yvXC9J4b3/SUZpuqYyYo6xdTQ6w4Dw v9xw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=kRFFQCAzPSqug/s9TuSfbcrQXz9M9tI/wJEAGRzzT/8=; b=tQKhROHynbiJziv4V/KMcxaPxNKRlcvEV/0EX84/PXta/baWhDpD9aPha8XxgtUOzW XQyNNE7jXIk4fz/XQAXfqSAUrIkUWTzGJEPcHzZs00W+3sG0a7IWDvh2UjjTN9imWTCZ 9WWvFl0rv6odF4QJ4GEbWVuM7DKMm9kVaWijGzNdxqXmP2aWbK+AMtp0D9U5SxoSdmEO iYK1Sy6Kh22Dbw6tZ34iyLAqlSRtvY5qaf3gnVWLfZkY/HEQ8WQ+JN7v7uwkF/Ai3Mfw IqSRPorIoVkppHjyvzuHoaVIlz2OH3PAciBgrr6pEiPC0NsSzS+/OOq5BW9hrqH4Yl0O rkmQ== X-Gm-Message-State: AOAM530jMKg53qeEbwfQfhhPOJX0OOPrOv+nD4CJUzWqmYseeaxIFosa LqPkCb0PtC3T/VUWV8kBbBzLKj42c/URTetKisqPRGX62jUVjg== X-Google-Smtp-Source: ABdhPJyM0kQbK2yM5Ny5tPX65MfC87nInLZk81+FlqvBwDL7INOVy1EOHvv2BDJoadkYcMLeC5vSE4RvEXkc1w6LvCk= X-Received: by 2002:adf:9f10:: with SMTP id l16mr24816631wrf.77.1599508251407; Mon, 07 Sep 2020 12:50:51 -0700 (PDT) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:187478 Archived-At: --00000000000023f78c05aebe89cb Content-Type: text/plain; charset="UTF-8" Greg Lucas 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. --00000000000023f78c05aebe89cb Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Greg Lucas <greg@glu= cas.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 re= produced 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-doub= le-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
> .ema= cs file contains:
>
> (custom-set-variables
> =C2=A0;; cu= stom-set-variables was added by Custom.
> =C2=A0;; If you edit it by = hand, you could mess it up, so be careful.
> =C2=A0;; Your init file = should contain only one such instance.
> =C2=A0;; If there is more th= an one, they won't work right.
> =C2=A0'(sentence-end-double-= space nil)
> =C2=A0'(user-full-name "Greg Lucas"))
&= gt; (custom-set-faces
> =C2=A0;; custom-set-faces was added by Custom= .
> =C2=A0;; If you edit it by hand, you could mess it up, so be care= ful.
> =C2=A0;; Your init file should contain only one such instance.=
> =C2=A0;; If there is more than one, they won't work right.
= > =C2=A0)
>
>
>
> From testing this I've fou= nd that at the time I call `load-theme` all
> customizations made for= the current session will get marked to be saved.
> Changes made afte= r load-theme behave as expected and do not get saved
> unless I expli= citly choose to save them.

This one is really tricky.

When sa= ving 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) =3D= =3D> nil
or have the user theme preference applied:
(caar (get sym= bol 'theme-value)) =3D=3D> 'user

So we shouldn't let = those combinations happen by accident.

The offending code is in cust= om-theme-recalc-variable: when there is a
custom theme that specifies a = value for the variable, it puts that into
the 'saved-value property.= =C2=A0 And custom-theme-recalc-variable is called
when enabling or disab= ling a theme.=C2=A0 So what happens in the recipe is:
- The user customi= zes some variable, so now:
(get VARIABLE 'theme-value) =3D=3D> ((= user SETTING))
(get VARIABLE 'saved-value) =3D=3D> nil

- T= hen the user loads a theme with M-x load-theme.=C2=A0 So we enable it, and<= br>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 'them= e-value) =3D=3D> ((user SETTING))
(get VARIABLE 'saved-value) =3D= =3D> (SETTING)

- If later in the session `custom-save-all' ru= ns, then we save that
setting by mistake.

The code that depends o= n that side-effect of
custom-theme-recalc-variable is the custom-initial= ize-* functions (at
least in custom.el).=C2=A0 Those functions need to k= now if there is one value
stashed for the variable, to set it accordingl= y when initializing it.
And we stash the value when we load the custom-f= ile or when loading a
theme.=C2=A0 So we are using the saved-value prope= rty for stashing this
value, which seems unfortunate to me.

Furth= ermore, 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:<= br>(get VARIABLE 'theme-value) =3D=3D> ((THEME THEME-SETTING))
(g= et VARIABLE 'saved-value) =3D=3D> (THEME-SETTING)

Since THEME= is not the user theme, we are not saving it in
`custom-save-all'.= =C2=A0 But, when we disable the theme we have:
(get VARIABLE 'theme-= value) =3D=3D> nil
(get VARIABLE 'saved-value) =3D=3D> (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
initia= lly void in our session.
(get VARIABLE 'theme-value) =3D=3D> ((TH= EME THEME-SETTING))
(get VARIABLE 'saved-value) =3D=3D> (THEME-SE= TTING)

And then Emacs finds the option.=C2=A0 Then:
VARIABLE =3D= =3D> THEME-SETTING
(get VARIABLE 'theme-value) =3D=3D> ((THEME= THEME-SETTING))
(get VARIABLE 'saved-value) =3D=3D> (THEME-SETTI= NG)

So, that shows that stashing the value worked.=C2=A0 But say we = customize the
variable for the session:
VARIABLE =3D=3D> OUR-SETTI= NG
(get VARIABLE 'theme-value) =3D=3D> ((user OUR-SETTING) (THEME= THEME-SETTING))
(get VARIABLE 'saved-value) =3D=3D> (THEME-SETTI= NG)

And we lose again.


I think that if we want to keep st= ashing the value under the
saved-value property, we could try to stash t= he value only if we need
to.=C2=A0 That is, if default-toplevel-value er= rors out, which would mean
custom-initialize-* will need to know if ther= e is a saved-value.=C2=A0 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.
--00000000000023f78c05aebe89cb--