bug#59937: 28.2; Bad defcustom behavior

[Drew Adams]

I was actually hoping that you'd see and reply to
this bug report, Mauro.  You're our defcustom expert!

Turns out that this is the 3rd time I've reported
this bug, not realizing I'd already reported it!

> The moment you add a match alternative that won't match the default
> value of a restricted-sexp widget (which is nil), then you should change
> the default value for the restricted-sexp widget.

I don't even see how/where that widget gets a default
value of nil.  I see `:match-alternatives '(functionp)',
and I see `:value 'ignore'.  I admit that I don't really
understand the code that implements `restricted-sexp'.
(And I'd think that I shouldn't really need to understand it.)
 
>  > (defcustom myvar ()
>  >   "..."
>  >   :group 'emacs
>  >   :type '(alist :key-type (string :tag "Alist key (string):")
>  >                 :value-type
>  >                 (plist :key-type
>  >                        (restricted-sexp :match-alternatives (keywordp)
>  >                                         :tag "Plist key (keyword)")
>  >                        :options (:x :y :z)
>  >                        :value-type (repeat string))))
>  >
>  > In this case the default value is nil, but the defcustom also
>  > specifies the type of plist values as keywordp.  I think this 
>  > definition should work fine.
> 
> You're looking at a different default value.
> The warning comes from Widget, and says that the default
> value for the restricted-sexp widget is not
> correct.  It's not talking about the default value
> for the user option.

Note that in bug #25152 I expressed my disagreement
with closing the bug - IMO, it's not fixed.

I don't understand how a defcustom should be bothering
with (working around) a default value for the widget
that's defined for `restricted-sexp'.

The default value of the option is the only default
value that should matter, no?  If the initial (i.e.
default) value of the option is nil, then the alist
is nil, which means it has no elements, which means
there are no plists.

So it's impossible to even speak about applying some
condition to a plist element.  Such a test (which is
what `restricted-sexp' defines) is never - can never
be, logically - applied to any plist element because
no such elements exist in this default case.

To me, this is just, well, a bug.  A bug in the
definition of widget `restricted-sexp', I guess (?).

But a priori I'd think the bug is not in the definition
of `restricted-sexp' but in the definition of anything
that uses it.  To me, widget `restricted-sexp' just
shouldn't apply at all in a context such as described in
this bug: there's _nothing to check_ with a predicate
that's used to check each list element - there are no
elements.

To me, `null' as a predicate doesn't apply either.
This isn't about testing whether some plist element
is nil.  Nothing about the plist should be checked,
because there's no plist!  There's certainly not a
plist with nil elements - now should predicate `null'
help here (but it does!)?

Even in bug #25152, which is simpler, I can't see
where a proper "fix" (workaround) by the defcustom
definer is to add predicate `null'.  Likewise,
adding `:value ignore'.  No predicate that tests
a value in a `repeat' list can possibly be a way to
validate or invalidate a `repeat' list that has no
elements at all - an empty list.

It's like saying that for (mapcar PRED ()) we need
to have a PRED such as `null' - makes no sense to me.
`mapcar' has to itself be defined so that any PRED
you provide isn't invoked when the list arg is ().

I guess I still don't get it.

And I can't tell whether you think there is a bug
or not.  The duplicate bugs were closed (by Lars),
after you tried to improve things by providing a
warning.  Though I suppose it was good to provide
the warning, I don't see that the bug is fixed at all.

And I'm (still) afraid that any user (including the
person writing the defcustom and testing it) won't
(1) understand what's going on and (2) be able to
figure out how to fix the `restricted-sexp' to work
around the problem.  Does always adding `null' take
care of it?

I just now tried the _two_ alternative workarounds
for `restricted-sexp' you showed in the context of
bug #25152: (1) add `:value ignore' or (2) add
predicate `null' to the list of predicates.

In the case of the example in this bug (#59937)
it looks like #1 doesn't work - you get the same
warning etc.  But #2 works.

> Examples 2-4 get the same warning once the user clicks the INS button.
> If you specify a valid default value for the restricted-sexp widget,
> then the warning is gone.  See also bugs #15689, #25152.

Expecting a defcustom definer to understand this
and figure out what a "valid default value for
the restricted-sexp widget" might be, is a bridge
too far, IMO.

At my present, poor state of understanding this,
about all I could tell someone is to add `null'
as a predicate.  I couldn't explain why or how
that works.  I can't see how/why any of the
predicates would/should get called if the value
of the option is ().

Anyway, I'm _grateful_ to you for pointing this
out (again), and for pointing to bugs #15689 and
#25152.  You're definitely the king of widgets,
and we're very lucky to have you involved.  Thx.

I am curious whether you think there's actually
a bug or not.  It's hard for me to believe that
we should expect _anyone_ defining a defcustom
(let alone anyone using Customize) to understand
the `restricted-sexp' widget, what it requires
wrt its "default value", and how to adjust a
defcustom to give it what it needs, to DTRT.