bug#59937: 28.2; Bad defcustom behavior

[Mauro Aranda <maurooaranda@gmail.com>]

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

 >> Oh, I think I see a way around that now.  I think the following will
 >> take care of:
 >>
 >> 1.  Being able to create the restricted-sexp (sub)widget even if the
 >> default value isn't valid.
 >> (Which I think it's one of your main points throughout the bug report)
 >>
 >> 2.  Being able to do it without prompting or whatsoever.
 >> (Which is one of my main points in this conversation).
 >
 > Fabulous!
 >
 >> When the restricted-sexp widget has to be created, if there's a valid
 >> default value we create it with that one (like I showed in my previous
 >> message), but if there's not we create it empty.
 >>
 >> Let me know if you agree with that.
 >
 > 100%.  I hope you can do it without too much trouble.
 > It will make a big difference, I think, including
 > perhaps in how much people make use of `restricted-sexp'.
 >
 >> As I've said, I don't think we need to (nor want to) prompt.  I think
 >> the prompt there is just an accident, and I would like to avoid it.
 >> Sorry if I sound stubborn about this, but I'm convinced that prompting
 >> at that time of the widget's creation can be really harmful.
 >
 > I was seeing prompting only as a necessity as long as the code
 > requires a value before it can create the UI field for the
 > `restricted-sexp'.  If you can dispense with that need then great!
 > Certainly it would be much better not to have any prompting
 > (especially not with just the default prompt).

Great! I'll work on it.  I hope I don't find any surprises.

 >> I've seen things like:
 >> (defcustom foo nil
 >>    "..."
 >>    :type '(repeat (function :value t)))
 >>
 >> And I would like to make more evident these kind of errors.  But if we
 >> find a way to cope with an invalid default value for the restricted-sexp
 >> widget, then it might be fine to remove it (I'm not so sure yet).
 >
 > I thought it already coped with invalid input.
 > I guess I was mistaken.  It definitely should.
 >
 > Generally, all Customize UI fields (including
 > buttons, checkboxes, etc.) do check the input
 > for validity, I think.  Not necessarily at the
 > time you edit but at least when you try to set
 > the value to what your editing resulted in.

Yes, Customize checks at the time of creation (and inserts "(mismatch)"
when the value isn't valid), and at the time of setting/saving. But the
Widget code does not check when creating the widget.

 >> Yes, thanks to your response I was able to see a way to create the
 >> editable field (with value ""), when there's no valid default value.
 >
 > Really glad I could contribute something to this,
 > by my incessant arguing/questioning. ;-)
 > I appreciate your working on this.  I doubt that
 > anyone else would try to tackle it.

:-)

 >>  > And definers ideally shouldn't need to specify
 >>  > default values for such fields - the set of
 >>  > predicates should be able to define what kind
 >>  > of UI field is needed.
 >>
 >> I'm not sure if I understand what you say here.  I don't think it's
 >> possible to figure out a good value to use as a default from the
 >> predicates: that's why my idea is about creating it with the empty
 >> string.
 >
 > Ah.  Maybe we do disagree, in the sense that I still
 > don't understand.
 >
 > Is there a _logical_ requirement that there be a
 > value, in order to create the editable field for
 > the `restricted-sexp'?  I don't think there should
 > be.
 >
 > That's different from the need for a value because
 > the current code works that way.
 >
 > But I really don't see why a value is needed.  All
 > the code needs to do is create an editable field
 > that expects text that satisfies the predicates,
 > no?  Of what (logical) use is the ("default") value?

I think you're right on point.  It's just that the code works that way,
and makes assumptions that there is always going to be a value (default
or edited).

 >> So, would you agree to creating the restricted-sexp widget with an empty
 >> editable field, in case the default value is not valid?
 >
 > In case it's missing, definitely.
 >
 > In case it's not valid?  I guess so, but in that
 > case it would be good to signal an error (somehow),
 > or a message saying that it's invalid and so will
 > be ignored (create the field without any value).
 >
 >> Then the need to provide a valid default value is not so strong anymore
 >> (but still should be encouraged, I think), and Customize can work better
 >> and more intuitively when there isn't a valid default value.
 >
 > It all sounds good to me.  Looking forward to
 > whatever you come up with.  Thx.

I'll see what I can do in the next days.  Thanks!