GNU bug report logs - #15214
24.3.50; `Revert This Session's Customization' does not do what it says

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15214 in the body.
You can then email your comments to 15214 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.50; `Revert This Session's Customization' does not do what it
 says
Date: Thu, 29 Aug 2013 22:10:41 -0700 (PDT)

1. emacs -Q

M-x customize-option backup-by-copying

Click Toggle, then State and Set For Current Session.
You have changed the value.

Click State and Revert This Session's Customization.

The current value does not change back to what it was before you changed
the setting for the session.  This session's customization was not
reverted.  However, even though the value was not changed, you see the
message "CHANGED outside Customize".

2. The State choice of Revert This Session's Customization is not even
documented at (emacs) `Changing a Variable'.  Other State actions are
documented.  So it's hard to tell whether #1 above is really a bug: it's
not even clear what this menu item is supposed to do.




In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
 of 2013-08-23 on ODIEONE
Bzr revision: 113986 rgm <at> gnu.org-20130823185841-zoy6h1qk433ibrlf
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/binary --enable-checking=yes,glyphs
 'CFLAGS=-O0 -g3' LDFLAGS=-Lc:/Devel/emacs/lib
 CPPFLAGS=-Ic:/Devel/emacs/include'

Message #8 received at 15214 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: 15214 <at> debbugs.gnu.org
Cc: Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#15214: 24.3.50; `Revert This Session's Customization' does
 not do what it says
Date: Mon, 30 Dec 2019 18:30:31 -0300

[Message part 1 (text/plain, inline)]

tags 15214 patch
thanks

Drew Adams <drew.adams <at> oracle.com> writes:

> 1. emacs -Q
>
> M-x customize-option backup-by-copying
>
> Click Toggle, then State and Set For Current Session.
> You have changed the value.
>
> Click State and Revert This Session's Customization.
>
> The current value does not change back to what it was before you changed
> the setting for the session.  This session's customization was not
> reverted.  However, even though the value was not changed, you see the
> message "CHANGED outside Customize".

I can reproduce this in latest master and emacs-27 branch.

> 2. The State choice of Revert This Session's Customization is not even
> documented at (emacs) `Changing a Variable'.  Other State actions are
> documented.  So it's hard to tell whether #1 above is really a bug: it's
> not even clear what this menu item is supposed to do.

Current (emacs) Changing a Variable says:
‘Revert This Session's Customizations’
     This restores the value of the variable to the last saved value, if
     there was one.  Otherwise it restores the standard value.  It
     updates the text accordingly.

The function that runs for that option is custom-variable-reset-saved,
and the doc string of custom-variable-reset-saved says something similar.

The attached patch fixes custom-variable-reset-saved to do what it says
it does when the variable has no previous saved value.

Best regards,
Mauro.

[Message part 2 (text/html, inline)]

[0001-Reset-to-the-standard-value-when-reverting-session-s.patch (text/x-patch, attachment)]

Message #13 received at 15214 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Mauro Aranda <maurooaranda <at> gmail.com>, 15214 <at> debbugs.gnu.org
Subject: RE: bug#15214: 24.3.50; `Revert This Session's Customization' does
 not do what it says
Date: Mon, 30 Dec 2019 13:52:39 -0800 (PST)

Thanks.

[(emacs) `Changing a Variable' was updated to mention 
this after the Emacs release for the bug report (24.3).]

Message #16 received at 15214 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: 15214 <at> debbugs.gnu.org
Subject: Re: bug#15214: 24.3.50;
 `Revert This Session's Customization' does not do what it says
Date: Tue, 31 Dec 2019 18:26:50 +0200

> From: Mauro Aranda <maurooaranda <at> gmail.com>
> Date: Mon, 30 Dec 2019 18:30:31 -0300
> 
> > Click State and Revert This Session's Customization.

You should instead click "Set to Backup Value".  The Revert button is
in general for variables you have saved for future sessions.

> The function that runs for that option is custom-variable-reset-saved,
> and the doc string of custom-variable-reset-saved says something similar.

The doc string is ambiguous, and the code definitely does NOT intend
to reset the value, just to remove the recorded setting, so that it
won't be saved in the custom file.  That code was installed in
response to a very similar bug report (bug#9509, except that it
complained that Emacs signals an error for a variable that was never
saved).  I very much doubt that Chong, who made that change, omitted
setting the value by mistake.

In general, there's a feature creep here: this menu item was
originally only for customized options that were saved during this
session.  That was lifted back then, and now we want also to change
the value, although another menu item exists to do just that.

> The attached patch fixes custom-variable-reset-saved to do what it says
> it does when the variable has no previous saved value.

I won't object too much to such a change, if you still think it's TRT
here after reading the above, but I wonder whether it would be cleaner
and safer to set just the value, and move the funcall out of the
if-else form, so that we'd have only one funcall, and it will be
inside ignore-errors.

> -	;; If there is no saved value, remove the setting.
> -	(custom-push-theme 'theme-value symbol 'user 'reset)
> +        (progn
> +	  ;; If there is no saved value, remove the setting.

The comment needs to be updated, since we now don't merely remove the
setting.

Thanks.

Message #19 received at 15214 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 15214 <at> debbugs.gnu.org
Subject: Re: bug#15214: 24.3.50; `Revert This Session's Customization' does
 not do what it says
Date: Wed, 1 Jan 2020 11:47:45 -0300

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Mauro Aranda <maurooaranda <at> gmail.com>
>> Date: Mon, 30 Dec 2019 18:30:31 -0300
>>
>> > Click State and Revert This Session's Customization.
>
> You should instead click "Set to Backup Value".  The Revert button is
> in general for variables you have saved for future sessions.

For user options that are of boolean type, that works well.  But for
other kinds of user options, there is a risk it won't work:

1. emacs -Q
2. M-x customize-variable dired-kept-versions
3. Edit the field to 3 and then Set for current session.
4. Now edit it to 4, and then Set for current session.

Now "Set to Backup Value" won't ever take you back to the state before
your session's customizations.

>> The function that runs for that option is custom-variable-reset-saved,
>> and the doc string of custom-variable-reset-saved says something similar.
>
> The doc string is ambiguous, and the code definitely does NOT intend
> to reset the value, just to remove the recorded setting, so that it
> won't be saved in the custom file.  That code was installed in

What part of the doc string do you find ambiguous? Personally, I was
baffled about the (themed or standard) part, because if there is a
theme value (other than the user theme), that value counts as
the saved value too.

As for the intentions of the code, a symbol with no 'saved-value property
set would be enough for the customization to not be saved in the custom
file, AFAICS.  It's true that it does not intend to reset the value, but
at least my reading of the doc string makes me think it does, the Emacs
manual says it does, and even though the use case is rare, I think it's
better to do it.

> response to a very similar bug report (bug#9509, except that it
> complained that Emacs signals an error for a variable that was never
> saved).  I very much doubt that Chong, who made that change, omitted
> setting the value by mistake.

In Bug#9509 the suggestion was to revert those settings to the
un-customized value.  I can't tell whether Chong was inclined to
take that suggestion or not, but I do think that we should take it.

> In general, there's a feature creep here: this menu item was
> originally only for customized options that were saved during this
> session.  That was lifted back then, and now we want also to change
> the value, although another menu item exists to do just that.

See the first part of my reply.  I don't think there is another menu
item in an emacs -Q session able to take one back to the uncustomized
value, reliably.

>> The attached patch fixes custom-variable-reset-saved to do what it says
>> it does when the variable has no previous saved value.
>
> I won't object too much to such a change, if you still think it's TRT
> here after reading the above, but I wonder whether it would be cleaner
> and safer to set just the value, and move the funcall out of the
> if-else form, so that we'd have only one funcall, and it will be
> inside ignore-errors.

I still think it would be a good addition to the code.  In case you are
OK with it, I attach a patch with moves the funcall outside of the
if-else form.

Best regards,
Mauro.

[Message part 2 (text/html, inline)]

[0001-Reset-to-the-standard-value-when-reverting-session-s.patch (text/x-patch, attachment)]

Message #22 received at 15214 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: 15214 <at> debbugs.gnu.org
Subject: Re: bug#15214: 24.3.50; `Revert This Session's Customization' does
 not do what it says
Date: Wed, 01 Jan 2020 18:36:34 +0200

> From: Mauro Aranda <maurooaranda <at> gmail.com>
> Date: Wed, 1 Jan 2020 11:47:45 -0300
> Cc: 15214 <at> debbugs.gnu.org
> 
> > You should instead click "Set to Backup Value".  The Revert button is
> > in general for variables you have saved for future sessions.
> 
> For user options that are of boolean type, that works well.  But for
> other kinds of user options, there is a risk it won't work:
> 
> 1. emacs -Q
> 2. M-x customize-variable dired-kept-versions
> 3. Edit the field to 3 and then Set for current session.
> 4. Now edit it to 4, and then Set for current session.
> 
> Now "Set to Backup Value" won't ever take you back to the state before
> your session's customizations.

"Set to Backup Value" reverts to the previous value, not to the
original one.  So this is working as intended.

> 
> >> The function that runs for that option is custom-variable-reset-saved,
> >> and the doc string of custom-variable-reset-saved says something similar.
> >
> > The doc string is ambiguous, and the code definitely does NOT intend
> > to reset the value, just to remove the recorded setting, so that it
> > won't be saved in the custom file.  That code was installed in
> 
> What part of the doc string do you find ambiguous?

The "reset" part.

> I still think it would be a good addition to the code.  In case you are
> OK with it, I attach a patch with moves the funcall outside of the
> if-else form.

It LGTM, thanks.

Message #25 received at 15214 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 15214 <at> debbugs.gnu.org
Subject: Re: bug#15214: 24.3.50; `Revert This Session's Customization' does
 not do what it says
Date: Tue, 7 Jan 2020 18:58:13 -0300

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> What part of the doc string do you find ambiguous?
>
> The "reset" part.

The best I can come up with is just saying:
"...; otherwise, set the variable to the standard value."
(If the change is applied, of course).

>> I still think it would be a good addition to the code.  In case you are
>> OK with it, I attach a patch with moves the funcall outside of the
>> if-else form.
>
> It LGTM, thanks.

Great.  If there are no other objections, I'll wait for someone to push
it.

[Message part 2 (text/html, inline)]

Message #30 received at 15214-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: 15214-done <at> debbugs.gnu.org
Subject: Re: bug#15214: 24.3.50; `Revert This Session's Customization' does
 not do what it says
Date: Sat, 11 Jan 2020 10:39:19 +0200

> From: Mauro Aranda <maurooaranda <at> gmail.com>
> Date: Tue, 7 Jan 2020 18:58:13 -0300
> Cc: 15214 <at> debbugs.gnu.org
> 
> >> I still think it would be a good addition to the code.  In case you are
> >> OK with it, I attach a patch with moves the funcall outside of the
> >> if-else form.
> >
> > It LGTM, thanks.
> 
> Great.  If there are no other objections, I'll wait for someone to push
> it.

Thanks, pushed to the emacs-27 branch, and closing the bug.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.