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

From: Drew Adams <[email-masked]>
Date: Sun, 10 Jul 2016 10:18:29 -0700 (PDT)

> > > > 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.