bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Noam Postavsky]

A trivial example:

(defcustom time (current-time-string)
  "the time"
  :type 'string)

Then try to M-x customize-options RET time RET, it will show with
state "CHANGED outside Customize." Similarly, doing <f1> v time RET
shows the "original value" as the current time, not the actual value
when `time' was defined.

See also http://debbugs.gnu.org/cgi/bugreport.cgi?bug=4755#25

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

Calling such an expression "non-pure" can be misleading.

The concept you are looking for is apparently just non-constant,
IOW, an expression whose value is not always the same.

IOW the expression is not referentially transparent.

But in Lisp non-pure often refers to or suggests side effects,
esp. mutation of structure.  That's not required for this bug
to manifest.  All that's needed is for the expression to not
necessarily return the same value each time it is evaluated. 

Anyway, yes, I agree that filing another bug for this
(separate from #4755) is good.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Fri, 8 Jul 2016 23:11:02 -0400
> 
> A trivial example:
> 
> (defcustom time (current-time-string)
>   "the time"
>   :type 'string)
> 
> Then try to M-x customize-options RET time RET, it will show with
> state "CHANGED outside Customize." Similarly, doing <f1> v time RET
> shows the "original value" as the current time, not the actual value
> when `time' was defined.

Why is this a bug?  Seems to be expected behavior to me.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[npostavs]

retitle 23926 defcustom with STANDARD=<non-constant-expression> gives confusing results
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Fri, 8 Jul 2016 23:11:02 -0400
>> 
>> A trivial example:
>> 
>> (defcustom time (current-time-string)
>>   "the time"
>>   :type 'string)
>> 
>> Then try to M-x customize-options RET time RET, it will show with
>> state "CHANGED outside Customize." Similarly, doing <f1> v time RET
>> shows the "original value" as the current time, not the actual value
>> when `time' was defined.
>
> Why is this a bug?  Seems to be expected behavior to me.

Yeah, it seems expected because you're familiar with the code.  But it
causes Emacs to claim the "original" value is different from what it
originally was, which seems nonsensical.

I wonder why Emacs saves only the original expression and not the
actual original value?

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 23926@debbugs.gnu.org
> Date: Sat, 09 Jul 2016 07:54:58 -0400
> 
> Yeah, it seems expected because you're familiar with the code.  But it
> causes Emacs to claim the "original" value is different from what it
> originally was, which seems nonsensical.

It's not nonsensical, it's what actually happens, AFAIK.

> I wonder why Emacs saves only the original expression and not the
> actual original value?

It does, but then it reevaluates at startup.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Noam Postavsky]

On Sat, Jul 9, 2016 at 8:31 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: npostavs@users.sourceforge.net
>> Cc: 23926@debbugs.gnu.org
>> Date: Sat, 09 Jul 2016 07:54:58 -0400
>>
>> Yeah, it seems expected because you're familiar with the code.  But it
>> causes Emacs to claim the "original" value is different from what it
>> originally was, which seems nonsensical.
>
> It's not nonsensical, it's what actually happens, AFAIK.

What actually happens is that Emacs goes back in time to change what
the original value was? Perhaps the dates in etc/future-bug are wrong
then ;)

>
>> I wonder why Emacs saves only the original expression and not the
>> actual original value?
>
> It does, but then it reevaluates at startup.

It seems it's also reevaluated every time the variable is looked at
with customize, or describe-variable.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Sat, 9 Jul 2016 08:55:48 -0400
> Cc: 23926@debbugs.gnu.org
> 
> On Sat, Jul 9, 2016 at 8:31 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: npostavs@users.sourceforge.net
> >> Cc: 23926@debbugs.gnu.org
> >> Date: Sat, 09 Jul 2016 07:54:58 -0400
> >>
> >> Yeah, it seems expected because you're familiar with the code.  But it
> >> causes Emacs to claim the "original" value is different from what it
> >> originally was, which seems nonsensical.
> >
> > It's not nonsensical, it's what actually happens, AFAIK.
> 
> What actually happens is that Emacs goes back in time to change what
> the original value was?

No, that's the plan for Emacs 42.  For now, we just compare to the
previous recorded value (computed at dump time, I presume).

> >> I wonder why Emacs saves only the original expression and not the
> >> actual original value?
> >
> > It does, but then it reevaluates at startup.
> 
> It seems it's also reevaluated every time the variable is looked at
> with customize, or describe-variable.

Quite possibly, we reevaluate a defcustom when in doubt.

The question is, given the above, do we need to actually do anything
with this report, except close it?

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Noam Postavsky]

On Sat, Jul 9, 2016 at 9:14 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> For now, we just compare to the
> previous recorded value (computed at dump time, I presume).

Dump time? This doesn't apply to non-preloaded libraries though, right?

>
>> >> I wonder why Emacs saves only the original expression and not the
>> >> actual original value?
>> >
>> > It does, but then it reevaluates at startup.
>>
>> It seems it's also reevaluated every time the variable is looked at
>> with customize, or describe-variable.
>
> Quite possibly, we reevaluate a defcustom when in doubt.
>
> The question is, given the above, do we need to actually do anything
> with this report, except close it?

Either really save the original value, or don't call it the "original
value". We can't use "default value" because that's already used for
the non-buffer local value, unfortunately. I think "standard value"
could work, though Drew said it was unclear.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Sat, 9 Jul 2016 09:48:18 -0400
> Cc: 23926@debbugs.gnu.org
> 
> On Sat, Jul 9, 2016 at 9:14 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > For now, we just compare to the
> > previous recorded value (computed at dump time, I presume).
> 
> Dump time? This doesn't apply to non-preloaded libraries though, right?

Right.

> > The question is, given the above, do we need to actually do anything
> > with this report, except close it?
> 
> Either really save the original value, or don't call it the "original
> value". We can't use "default value" because that's already used for
> the non-buffer local value, unfortunately. I think "standard value"
> could work, though Drew said it was unclear.

Why change anything in the wording at all?  It won't really change
what is being done, and won't prevent any confusion, because all this
"standard", "original", "default" etc. are not well defined anyway.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> Either really save the original value, or don't call it the "original
>> value". We can't use "default value" because that's already used for
>> the non-buffer local value, unfortunately. I think "standard value"
>> could work, though Drew said it was unclear.
>
> Why change anything in the wording at all?  It won't really change
> what is being done, and won't prevent any confusion, because all this
> "standard", "original", "default" etc. are not well defined anyway.

I had a look at the docs; seems to me "standard" is used consistently,
so it makes sense to change to that.  Docstring of defcustom:

    STANDARD is an expression specifying the variable’s standard
    value.  It should not be quoted.  It is evaluated once by
    ‘defcustom’, and the value is assigned to SYMBOL if the variable
    is unbound.  The expression itself is also stored, so that
    Customize can re-evaluate it later to get the standard value.
    DOC is the variable documentation.

Elisp manual description of defcustom:

    The argument STANDARD is an expression that specifies the standard
    value for OPTION. ...

    The expression STANDARD can be evaluated at various other times,
    too—whenever the customization facility needs to know OPTION’s
    standard value.  So be sure to use an expression which is harmless
    to evaluate at any time.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 23926@debbugs.gnu.org
> Date: Mon, 11 Jul 2016 23:26:25 -0400
> 
> I had a look at the docs; seems to me "standard" is used consistently,
> so it makes sense to change to that.  Docstring of defcustom:

I'm in favor of using consistent terminology, but this:

>     STANDARD is an expression specifying the variable’s standard
>     value.

is just tautology: it doesn't really explain what is STANDARD, except
by using the same word, which is too general to explain itself.

So if we want to use this, and consider that an improvement, I think
it would be good to explain some more what "standard value" means or
is.

Thanks.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

> Either really save the original value, or don't call it the "original
> value". We can't use "default value" because that's already used for
> the non-buffer local value, unfortunately. I think "standard value"
> could work, though Drew said it was unclear.

If it is what custom calls the "standard value", which is what
you get when you `Reset to Standard Setting', then I don't
think it is wrong to use that name here.

But I think that `C-h v' - or at a minimum somewhere in the
manual - should point out what is happening here:

The value shown is the result of re-evaluating, in the current
context, the original expression that was used to define the
initial value.

Each part of that sentence is important to understanding what
this is.  We should not just suppose that all of that is
understood by a user, just by providing a label such as
"standard value" (let alone a misleading label such as
"original value").

It's OK to just provide a (reasonable) label, I guess, _if_
it is clearly defined somewhere in the doc.

I really think that it would be good to show in `C-h v'
(also), the original expression.  That is available in
Customize (good), by choosing `Show Saved Lisp Expression'.

But if we are going to show, in `C-h v', what is currently
called "original value" there, then we should (1) use a
better name for it (e.g. "standard value") and (2) provide
also the expression that it was re-evaluated from.

As for how to show that expression: if small, show it inline;
if large, show it on demand (e.g. clicking an expand button).

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

> Why change anything in the wording at all?  It won't really change
> what is being done, and won't prevent any confusion, because all this
> "standard", "original", "default" etc. are not well defined anyway.

Maybe you mean that they have not been well defined in our help
for the user?  Because the standard value is well defined in
Customize, and it is referred to as such in the Customize UI.
("Original" and "default" are admittedly not so well defined.)

What's missing is to call it by the same name in `C-h v'.
And to provide some description/explanation in the doc, if it
is not there now (I haven't searched just now).

IOW, let's try to be clear with the labelling in `C-h v' -
consistent with the names used in Customize.  And let's try to
let users of `C-h v' get more info about what they're looking
at, to dispel confusion and answer questions.

I think we should also have `C-h v' provide the underlying
Lisp expression, at least on demand, just as Customize does.
It's not great to show only a value without any indication
of what it comes from.

As for whether to call the value shown "standard value":
IIUC, the standard value is:

;;    the value given in the 'defcustom' declaration.
;;    It is stored in the 'standard-value' property of the
;;    option, in a cons-cell whose car evaluates to the standard
;;    value.

That wording is maybe not perfect.  But IIUC, the value of the
`standard-value' property is not the "standard value".  Instead,
it is a cons whose car _evaluates_ to the standard value.  Its
car is, I guess, the original Lisp expression from the defcustom.

That is what needs to be made clear to users, I think, when
showing them a value.  Let them know that it is called the
"standard value", and it is the result of re-evaluating, in
the current context, the defining Lisp sexp for the option
(which is used in the defcustom). 

All of this is important for clarity.  In particular, I think
it is important that users understand the following, which is
I guess what is behind Eli saying that the behavior is as
expected:

;; The reason for storing values unevaluated: This is so you can have
;; values that depend on the environment.  For example, you can have a
;; variable that has one value when Emacs is running under a window
;; system, and another value on a tty.  Since the evaluation is only done
;; when the variable is first initialized, this is only relevant for the
;; saved (and standard) values, but affect others values for
;; compatibility.

The premise of that last sentence is wrong, of course.  It is done
each time you use `C-h v' - to show you the "original" value.

But the main point here is that it is a _feature_, not a bug, that
the "standard value" is recomputed at any time from the original sexp.
Why/how this is a feature is explained well in that paragraph.

But without such an explanation, and especially just showing a
value in `C-h v' and calling it the "original" value, we hurt
instead of help users.

;; You can see (and modify and save) this unevaluated value by selecting
;; "Show Saved Lisp Expression" from the Lisp interface.  This will
;; give you the unevaluated saved value, if any, otherwise the
;; unevaluated standard value.

And that's the other piece that helps understanding.  I think
`C-h v' should show users that Lisp sexp - at least on demand.

That will also help understanding of the standard value that is
shown (and should be labeled as such): `C-h v' can say that this
is the result of re-evaluating the Lisp sexp.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

Note, BTW, that this erroneous display of "original value"
by `C-h v' is not something that has been in Emacs long.

It was added in Emacs 24.1.  It should have been corrected
sooner than now, but it wasn't.  Now is a good time.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

> IOW, let's try to be clear with the labelling in `C-h v' -
> consistent with the names used in Customize.  And let's try to
> let users of `C-h v' get more info about what they're looking
> at, to dispel confusion and answer questions.
> 
> I think we should also have `C-h v' provide the underlying
> Lisp expression, at least on demand, just as Customize does.
> It's not great to show only a value without any indication
> of what it comes from.
...
> That is what needs to be made clear to users, I think, when
> showing them a value.  Let them know that it is called the
> "standard value", and it is the result of re-evaluating, in
> the current context, the defining Lisp sexp for the option
> (which is used in the defcustom).
> 
> All of this is important for clarity.  In particular, I think
> it is important that users understand the following, which is
> I guess what is behind Eli saying that the behavior is as
> expected:
> 
> ;; The reason for storing values unevaluated: This is so you can have
> ;; values that depend on the environment.  For example, you can have a
> ;; variable that has one value when Emacs is running under a window
> ;; system, and another value on a tty.  Since the evaluation is only done
> ;; when the variable is first initialized, this is only relevant for the
> ;; saved (and standard) values, but affect others values for
> ;; compatibility.
> 
> The premise of that last sentence is wrong, of course.  It is done
> each time you use `C-h v' - to show you the "original" value.
> 
> But the main point here is that it is a _feature_, not a bug, that
> the "standard value" is recomputed at any time from the original sexp.
> Why/how this is a feature is explained well in that paragraph.
> 
> But without such an explanation, and especially just showing a
> value in `C-h v' and calling it the "original" value, we hurt
> instead of help users.
> 
> ;; You can see (and modify and save) this unevaluated value by selecting
> ;; "Show Saved Lisp Expression" from the Lisp interface.  This will
> ;; give you the unevaluated saved value, if any, otherwise the
> ;; unevaluated standard value.
> 
> And that's the other piece that helps understanding.  I think
> `C-h v' should show users that Lisp sexp - at least on demand.
> 
> That will also help understanding of the standard value that is
> shown (and should be labeled as such): `C-h v' can say that this
> is the result of re-evaluating the Lisp sexp.

Here are a couple proposals for how to fix the `C-h v' part of
this bug.  Others are welcome.

1. Not print the "original value" at all, as was the case before
   Emacs 24.  Let users get such info from Customize.

2. Like #1, but give users a hint that such info is in fact
   available from Customize.  My suggestion here would be to not
   only remove printing the "original value" but to change the text
   "You can customize this variable.", where `customize' is a link to
   Customize, with this text, all of it a link with the same target:

      Customize or inspect

   (or possibly "Inspect or customize").  The point is for the
   link text to indicate that the target (Customize for the option)
   is not only for changing the value but also for finding out more
   about the option and its customization.

3. Like #1 and #3, but also provide a (toggle) link to show the
   defining Lisp sexp for the default value or, if it is shown,
   to reevaluate it and show the result:

      Show Lisp sexp defining the default value

   (if not shown) and

      Reevaluate

   (if shown - displayed just above it, in place of "Show Lisp sexp
   defining the default value").

I think any of these would improve the `C-h v' doc, especially for
this situation where the Lisp sexp can return different values.

If you decide to go for any of these approaches I could work on
a patch.

(Note that this mail is only about the `C-h v' part of the bug.
It does not address the part that concerns how the Customize UI
talks about the state - see my previous message about that part.)

bug#23926: defcustom with STANDARD=<non-constant-expression> gives confusing results

[Mauro Aranda]

I took a look at this Bug Report and all of the discussion.

I do agree with Eli that passing to default the following
(current-time-string) is quite nonsensical, but I understand that the
reproducer was just to show a way to see some inconsistency happening.

So I set out to look if there are occurrences like that in current Emacs
sources.  And there is one comparable:

(defcustom archive-tmpdir
   ;; make-temp-name is safe here because we use this name
   ;; to create a directory.
   (make-temp-name
    (expand-file-name (if (eq system-type 'ms-dos) "ar" "archive.tmp")
              temporary-file-directory))
   "Directory for temporary files made by `arc-mode.el'."
   :type 'directory)

That expression will evaluate to something different every time.  So, to
see the problems reported here, in actual Emacs source code:
emacs -Q
(require 'arc-mode)
C-h v archive-tmpdir
See that "the original value", as claimed incorrectly by C-h v (I agree
with Drew here about this terminology being non-accurate) is different
from the value of archive-tmpdir.

Kill the *Help* buffer and do again:
C-h v archive-tmpdir
The value of archive-tmpdir is the same, of course, but not "the
original value".

M-x customize-option RET archive-tmpdir
will show, as reported, that archive-tmpdir was changed outside of
Customize.  That's not true, it's just that the standard-value changed,
and there's no support for that.  Those two things are different to me.


I'm not saying that there's need to code something in Custom to handle these
cases.  At least, I'm not yet convinced.  But the case of archive-tmpdir
shows us, I think, that something needs to be done.  For example, change
the standard value to something that doesn't change that often, or
use the exact same form but wrap the defcustom in a let form, so that
a constant expression gets saved as the standard-value.

bug#23926: defcustom with STANDARD=<non-constant-expression> gives confusing results

[Mauro Aranda]

In case it wasn't clear in my previous message, I'm focusing on the
Customize part of the problem here.  I went through a lot of the
defcustoms we have in source, and found out that there are many, so
many, that can potentially show this inconsistency.  The one that's
comparable to the test case given is archive-tmpdir, but there are
others.  I've classified them in 6 categories, as:

- Standard value changes every time its standard form is evaluated
This is archive-tmpdir.

- Standard value depends on special conditions in an Emacs session
For example, highlight-changes-colors depends on the background-mode
frame parameter.  I've seen it's somewhat common to use different themes
(light and dark) in a session, so using a frame-parameter query as
standard is not optimal.  There are other cases like erc-nicks-bg-color,
vc-annotate-background-mode, etc.

- Standard value depends on some external (usually OS-related) condition
These are mostly defcustoms that try to find a program available.
Examples are remote-shell-program, wallpaper-command, etc.  The standard
values could change, potentially, if some program gets installed while
an Emacs session is active.

- Standard value depends on a built-in option
Take for example ido-case-fold, or ibuffer-case-fold-search.  If those
options get defined and after that the user customizes case-fold-search,
it will exhibit the inconsistency presented in this bug report.  What's
worse, is that the current session may show a different behavior than a
new session, if the user saves the customization for case-fold-search.

- Standard value depends on an option from the same package
gnus-*-directory options are a good example here.

- Standard value depends on another's package option
For example, erc-button-url-regexp that depends on
browse-url-button-regexp.

- Standard value depends on finding an Emacs-related file
This is usually detected with a call to locate-user-emacs-file with two
arguments, trying to keep backward-compatibility with an older filename.
For example, bookmark-default-file, diary-file, timeclock-file, etc.

I left this for last, because while it might potentially show the
problem, it should be rare enough to consider it a non-issue.


That's a summary of what I've found.  I would like to find a way to make
the Customize part of this problem less confusing (or better, no
confusing at all), keeping in mind backward-compatibility, of course.  I
don't have any clear ideas yet.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

All I can say is that I disagree.  So be it.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

> > 2. Is it not a bug that Customize tells you that the value
> >    was changed outside Customize?  In what way was it
> >    changed outside Customize?  In fact, it was not even
> >    changed.
> 
> It was changed, 

The option value was changed?  I don't think so.

The standard value (labeled "original" in `C-h v') is changed
each time the sexp is evaluated.  But the option value is not.

The option value was not changed at all in the recipe Noam gave.
It was and remained exactly what it was from the defcustom.
The mere fact of entering Customize did not change its value,
and nothing else changed its value.  It still has the original
value from when the defcustom was evaluated.

> because each time the sexp is evaluated it yields a
> different value.

See above.

> "Outside Customize" means not by the user who is typing values
> into the Custom buffer and saves those values by using the
> "set state" menu.

Correct.  And nothing changed the option value at all.  Not
that way or any other way.  It remains as it was from defcustom.

> > How about the reverse: Why do you think this is not a bug?
> 
> See above.

See above.  Do you still think this is not a bug?

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> Date: Sat, 9 Jul 2016 14:59:06 +0000 (UTC)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: npostavs@users.sourceforge.net, 23926@debbugs.gnu.org
> 
> > > 2. Is it not a bug that Customize tells you that the value
> > >    was changed outside Customize?  In what way was it
> > >    changed outside Customize?  In fact, it was not even
> > >    changed.
> > 
> > It was changed, 
> 
> The option value was changed?  I don't think so.

Yes, it was changed, because the value returned by the function
changes each time it's called.

> See above.  Do you still think this is not a bug?

Of course, I do.  Maybe you don't realize how many times Emacs
evaluates the value of a defcustom, but I do.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:
> Of course, I do.  Maybe you don't realize how many times Emacs
> evaluates the value of a defcustom, but I do.

What about making Emacs evaluate it less? e.g. replace occurences of
(eval (car (get var 'standard-value))) with

(or (get var 'original-value)
    (let ((val (eval (car (get var 'standard-value)))))
      (put var 'original-value val)
      val))

Wrapped in a function of course, call it custom-get-standard-value?

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: Drew Adams <drew.adams@oracle.com>,  23926@debbugs.gnu.org
> Date: Sat, 09 Jul 2016 16:48:23 -0400
> 
> > Of course, I do.  Maybe you don't realize how many times Emacs
> > evaluates the value of a defcustom, but I do.
> 
> What about making Emacs evaluate it less? e.g. replace occurences of
> (eval (car (get var 'standard-value))) with
> 
> (or (get var 'original-value)
>     (let ((val (eval (car (get var 'standard-value)))))
>       (put var 'original-value val)
>       val))

What will that do to the likes of custom-reevaluate-setting?

FWIW, I wouldn't try making any such changes in this area.  The number
of evaluations and the precise triggers for evaluating a defcustom is
a fragile setup, and I'd hate breaking it.  Certainly not for a
marginal use case such as the one in this report.  In effect, whoever
uses current-time-string as a defcustom's value tells Emacs that the
value is not important, because the programmer has no idea when in the
process of building and restarting Emacs will the value be taken.  Why
does it make sense to rock the boat in this sensitive area for such
use cases?

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

> whoever uses current-time-string as a defcustom's value tells Emacs that
> the value is not important, because the programmer has no idea when in
> the process of building and restarting Emacs will the value be taken.
> Why does it make sense to rock the boat in this sensitive area for such
> use cases?

Obviously, `(current-time)' was an example, to demo show the problem.

It can sometimes make a lot of sense for a defcustom to use a sexp
that might not return the same result when reevaluated.

The original bug, from which this report is an offshoot, was #4755.
The example there used this defcustom sexp: `(copy-sequence foo)'.

And in the context of the using code there is nothing wrong with
such a sexp: the intention is really to use, as default value, a
(new) list whose elements are the (exact same) elements as those
in the list `foo'.

And yes, this (intentionally) means that the user option, `toto',
that has this value can share the objects that are its initial
elements.  Nothing wrong with using such an initial value for a
user option.

The problem is not with being able to make use of such a sexp for
the default value.  The problem is with how Emacs talks about the
state of the option value.  It miscommunicates to users.  That's
what this bug is about: how Emacs talks about what is going on.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> Date: Sun, 10 Jul 2016 10:18:27 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 23926@debbugs.gnu.org
> 
> It can sometimes make a lot of sense for a defcustom to use a sexp
> that might not return the same result when reevaluated.

One way to do that while avoiding the issue at hand is to define a
'set' function to do the job, instead of doing it explicitly in the
initialization value.

> The original bug, from which this report is an offshoot, was #4755.
> The example there used this defcustom sexp: `(copy-sequence foo)'.
> 
> And in the context of the using code there is nothing wrong with
> such a sexp: the intention is really to use, as default value, a
> (new) list whose elements are the (exact same) elements as those
> in the list `foo'.

I guess it's crystal-clear now what's wrong with such a sexp.

> The problem is not with being able to make use of such a sexp for
> the default value.  The problem is with how Emacs talks about the
> state of the option value.  It miscommunicates to users.  That's
> what this bug is about: how Emacs talks about what is going on.

Emacs says the truth: the value of the defcustom was changed behind
Customize's back.

And since I've already said all that once before, let's stop going in
circles.  Nothing wrong with agreeing to disagree.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

> > (defcustom time (current-time-string) "the time" :type 'string)
> >
> > Then try to M-x customize-options RET time RET, it will show with
> > state "CHANGED outside Customize." Similarly, doing <f1> v time RET
> > shows the "original value" as the current time, not the actual value
> > when `time' was defined.
> 
> Why is this a bug?  Seems to be expected behavior to me.

Noam can provide his explanation.  For my part:

1. The "original value" shown by `C-h v' is not the original
   value.  In fact, it might never have been the value of the
   variable, and it might never become its value.

   This much is a doc problem.  What is shown is the result
   of evaluating, in the current context, the original,
   defining Lisp sexp.

   And if this is really what we want to show then it would
   be good to also show what that Lisp sexp is.  If it is
   large then provide a button/link to show it completely.

   And it might be good to also show the actual original
   value, i.e., the value when the variable was first set,
   however it was set.  (The original value could have
   been set without evaluating the original Lisp sexp of
   the defcustom.)

2. Is it not a bug that Customize tells you that the value
   was changed outside Customize?  In what way was it
   changed outside Customize?  In fact, it was not even
   changed.

How about the reverse: Why do you think this is not a bug?
What is expected about the behavior "changed outside" or
the behavior of saying that the "original value" is something
different from the original value?

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> Date: Sat, 9 Jul 2016 14:09:17 +0000 (UTC)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 23926@debbugs.gnu.org
> 
> 2. Is it not a bug that Customize tells you that the value
>    was changed outside Customize?  In what way was it
>    changed outside Customize?  In fact, it was not even
>    changed.

It was changed, because each time the sexp is evaluated it yields a
different value.  "Outside Customize" means not by the user who is
typing values into the Custom buffer and saves those values by using
the "set state" menu.

> How about the reverse: Why do you think this is not a bug?

See above.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Drew Adams]

> > > > 2. Is it not a bug that Customize tells you that the value
> > > >    was changed outside Customize?  In what way was it
> > > >    changed outside Customize?  In fact, it was not even
> > > >    changed.
> > >
> > > It was changed,
> >
> > The option value was changed?  I don't think so.
> 
> Yes, it was changed, because the value returned by the function
> changes each time it's called.

What function?  And what occurrence of calling it do you think
is responsible for this characterization of the value having
been changed outside Customize?

The fact is that the user did NOT change the value outside
customize.  And in fact, the value has NOT been changed.
It is what it was when the defcustom was evaluated.

The responsible code is `custom-variable-state', specifically
this part:

(setq tmp (get symbol 'standard-value))
(if (condition-case nil
        (and (equal value (eval (car tmp))) (equal comment nil))
      (error nil))
    'standard
  'changed)

That tests whether the current value (var VALUE here), which
in this case came from (default-value 'time), is equal to
the result of RE-evaluating the defining defcustom sexp,
(current-time).  And of course it is not equal, because
time passes...

The reason it is not unequal is NOT because something has
changed the option value outside Customize.  The option
value has not been changed at all.  What "changes" here is
the result of evaluating the initial sexp.

IOW, the "changed-outside-Customize" test used is too simplistic.  

Note that the code does try to correct its own logic in some
cases - for example, in this case:

;; The value was originally set outside
;; custom, but it was set to the standard
;; value (probably an autoloaded defcustom).

This but shows another case where its too-simplistic logic
trips it up, but this case is not being handled (compensated
for).

Nothing, including anything the user has done, has changed
the value outside Customize.  But the customize code is, so
far, unable to recognize that.

The code blithely assumes that evaluating what `custom-get'
returns represents the original value, whereas what it returns
is the result of RE-evaluating the original sexp.  That is
precisely the point of this bug.

The code correctly compensates in the case mentioned in
the comment cited above.  But it does not compensate in
the case demonstrated by the simple recipe Noam provided:

(defcustom time (current-time-string) "the time" :type 'string)

A _single_ evaluation of that defcustom should not throw
Customize off into thinking that the value has been changed
outside Customize.  And that is what is happening, because
its determination of "changed outside Customize" is too
simplistic.

> > See above.  Do you still think this is not a bug?
> 
> Of course, I do.  Maybe you don't realize how many times
> Emacs evaluates the value of a defcustom, but I do.

Please don't patronize us.  Everyone respects your understanding
of Emacs and Customize, but in this case I think you are wrong.

It is not a question of "how many times Emacs evaluates the
value of a defcustom".  It is about Emacs interpreting a
difference in the value returned by evaluating the defcustom
defining sexp from the current value as always representing a
change in the value of the variable (and outside Customize, to
boot).

I think we understand what is happening.  For us, telling the
user that the value has CHANGED from its original setting is
clearly wrong, since the VALUE has not changed.

And saying that it was changed outside Customize is doubly
wrong, since no user code or user action has done anything
to the value anywhere, including outside Customize.  This is
Customize stepping stepping on its own feet, and as a result
misleading users.

As for _fixing_ this part of the bug (the misleading state):

I don't see a solution other than doing either of these, but
other ideas are welcome:

1. Save also the original _value_ and compare the current
   value with that, instead of with the result of reevaluating
   the standard-value sexp.

2. Try to better characterize the state to users.  Instead
   of calling it changed-outside-customize, somehow indicate
   what it really means: the current value is not the same
   as what you get by reevaluating the defining sexp.

And then there is the other part of this bug: what to do for
`C-h v'.  I'll speak to that in a separate reply.

bug#23926: defcustom with STANDARD=<non-pure-expression> gives confusing results

[Eli Zaretskii]

> Date: Sun, 10 Jul 2016 10:18:29 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: npostavs@users.sourceforge.net, 23926@debbugs.gnu.org
> 
> > > The option value was changed?  I don't think so.
> > 
> > Yes, it was changed, because the value returned by the function
> > changes each time it's called.
> 
> What function?

current-time-string, of course.

> And what occurrence of calling it do you think is responsible for
> this characterization of the value having been changed outside
> Customize?

The second one.

> The fact is that the user did NOT change the value outside
> customize.

The message doesn't say it was the user.  Emacs doesn't know who
changed the value.

> And in fact, the value has NOT been changed.

Of course, it has changed.  Every time current-time-string is called
it returns a different value.  A defcustom's value is evaluated at
least twice, and in this case the second call yields a different
value.  That's why you see the note about changing.

> It is what it was when the defcustom was evaluated.
> 
> The responsible code is `custom-variable-state', specifically
> this part:
> 
> (setq tmp (get symbol 'standard-value))
> (if (condition-case nil
>         (and (equal value (eval (car tmp))) (equal comment nil))
>       (error nil))
>     'standard
>   'changed)
> 
> That tests whether the current value (var VALUE here), which
> in this case came from (default-value 'time), is equal to
> the result of RE-evaluating the defining defcustom sexp,
> (current-time).  And of course it is not equal, because
> time passes...
> 
> The reason it is not unequal is NOT because something has
> changed the option value outside Customize.  The option
> value has not been changed at all.  What "changes" here is
> the result of evaluating the initial sexp.
> 
> IOW, the "changed-outside-Customize" test used is too simplistic.  

No, it isn't.  It does its job.  If you want to avoid the note, if the
note annoys you, don't write such code.

> The code blithely assumes that evaluating what `custom-get'
> returns represents the original value, whereas what it returns
> is the result of RE-evaluating the original sexp.  That is
> precisely the point of this bug.

There's no bug.  This is how this stuff is supposed to work.  I'm not
going to endorse any significant changes there because of such
marginal use cases.