bug#23975: 25.0.94: defcustom error message is wrong when :type field has a :match attribute

[Lars Ingebrigtsen]

Robert Weiner <rsw@gnu.org> writes:

> Given a defcustom like:
>
> (defcustom bounded-num 999
>   "Positive, bounded number"
>   :type '(integer :match (lambda (widget value) (and (integerp value)
> (> value 0)
>       (< value 1000)))))
>
> When this variable is customized and a value of -5 is entered, the
> match function fails
> and the error signaled is:
>
>   (error "This field should contain an integer")
>
> which is wrong and not helpful.  Instead the error should display what
> the match function is and that the value failed to match.
>
> Secondarily, it would be nice if the type were checked before the match
> function were applied so that one did not need to add the (integerp
> value) test into the match function.

(I'm going through old Emacs bug reports that haven't received any
response.)

Both sound like good ideas, but the code here is rather convoluted.

So, for your defcustom (or "widget" at this point):

(widget-get w :match)
=> (lambda (widget value) (and (integerp value) (> value 0) (< value 1000)))

If that fails, then we get the error with

(widget-get w :type-error)
=> "This field should contain an integer"

So far so bad -- this means that custom doesn't actually call the
integerp check at all for defcustoms with an explicit :match.

Here's another defcustom without a custom :match:

(widget-get w2 :match)
=> widget-restricted-sexp-match

and that function does

(widget-get w2 :match-alternatives)
=> (integerp)

and then calls `integerp'.  Your defcustom also has this, but it's never
called:

(widget-get w :match-alternatives)
=> (integerp)

So this is all rather a mess.  It seems obvious that
(widget-get widget :match-alternatives) should always be called, but
it's not if you have an explicit :match, and my guess that doing so
might well break a lot of stuff.

As for the error message, we can't really fix that trivially either,
because you may have said :match widget-restricted-sexp-match or the
like, and then the error message is correct.  It sounds unlikely,
though, and we could add a hack that says that if :match is
widget-restricted-sexp-match, then we don't output the standard error
message but instead what's actually in :match, but that's...  hacky?

But possible.  Anybody have an opinion?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no