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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 23975 in the body.
You can then email your comments to 23975 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Robert Weiner <rsw <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.94: defcustom error message is wrong when :type field has a
 :match attribute
Date: Wed, 13 Jul 2016 16:03:28 -0400

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.

Bob

In GNU Emacs 25.0.94.1 (x86_64-apple-darwin13.4.0, NS appkit-1265.21
Version 10.9.5 (Build 13F1603))
 of 2016-05-17 built on builder10-9.local
Windowing system distributor 'Apple', version 10.3.1404
Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp''

Configured features:
NOTIFY ACL LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Custom

Message #8 received at 23975 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Robert Weiner <rsw <at> gnu.org>
Cc: rswgnu <at> gmail.com, 23975 <at> debbugs.gnu.org
Subject: Re: bug#23975: 25.0.94: defcustom error message is wrong when :type
 field has a :match attribute
Date: Sun, 28 Jul 2019 13:12:25 +0200

Robert Weiner <rsw <at> 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

Message #11 received at 23975 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: rswgnu <at> gmail.com, 23975 <at> debbugs.gnu.org, Robert Weiner <rsw <at> gnu.org>
Subject: Re: bug#23975: 25.0.94: defcustom error message is wrong when :type
 field has a :match attribute
Date: Sun, 28 Jul 2019 13:49:32 +0200

On Jul 28 2019, Lars Ingebrigtsen <larsi <at> gnus.org> wrote:

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

That's because the :match overrides the parent :match (defined by
restricted-sexp).  That's how OOP is working in general, I suppose.

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

:match-alternatives is only used by widget-restricted-sexp-match, which
is overridden by the custom :match.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Message #14 received at 23975 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: rswgnu <at> gmail.com, 23975 <at> debbugs.gnu.org, Robert Weiner <rsw <at> gnu.org>
Subject: 25.0.94: defcustom error message is wrong when :type field has a
 :match attribute
Date: Fri, 4 Sep 2020 08:28:03 -0300

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

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

I wonder if we could just document the :type-error property.  So
anybody that uses a custom :match function with additional checks can
put there the information they like to show the user when something goes
wrong.  So the defcustom posted would be something like:

(defcustom bounded-num 999
  "Positive, bounded number"
  :type '(integer :match (lambda (widget value) (and (integerp value)

                              (> value 0)

       (< value 1000)))
                        :type-error "Value should be an integer between 0
and 1000"))

That's easy, and would solve the main problem here.  WDYT?

[Message part 2 (text/html, inline)]

Message #17 received at 23975 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: rswgnu <at> gmail.com, 23975 <at> debbugs.gnu.org, Robert Weiner <rsw <at> gnu.org>
Subject: Re: 25.0.94: defcustom error message is wrong when :type field has
 a :match attribute
Date: Fri, 04 Sep 2020 14:12:44 +0200

Mauro Aranda <maurooaranda <at> gmail.com> writes:

> I wonder if we could just document the :type-error property.  So
> anybody that uses a custom :match function with additional checks can
> put there the information they like to show the user when something goes
> wrong.  So the defcustom posted would be something like:
>
> (defcustom bounded-num 999
>   "Positive, bounded number"
>   :type '(integer :match (lambda (widget value) (and (integerp value)
>                                                                                   (> value 0)
>                                                                                   (< value 1000)))
>                         :type-error "Value should be an integer between 0 and 1000"))
>
> That's easy, and would solve the main problem here.  WDYT?

I think that's a very good idea.

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

Message #20 received at 23975 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Robert Weiner <rsw <at> gnu.org>, rswgnu <at> gmail.com,
 Mauro Aranda <maurooaranda <at> gmail.com>, 23975 <at> debbugs.gnu.org
Subject: Re: bug#23975: 25.0.94: defcustom error message is wrong when :type
 field has a :match attribute
Date: Fri, 4 Sep 2020 09:48:01 -0300

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Mauro Aranda <maurooaranda <at> gmail.com> writes:
>
>> I wonder if we could just document the :type-error property.  So
>> anybody that uses a custom :match function with additional checks can
>> put there the information they like to show the user when something goes
>> wrong.  So the defcustom posted would be something like:
>>
>> (defcustom bounded-num 999
>>   "Positive, bounded number"
>>   :type '(integer :match (lambda (widget value) (and (integerp value)
>>
          (> value 0)
>>
          (< value 1000)))
>>                         :type-error "Value should be an integer between
0 and 1000"))
>>
>> That's easy, and would solve the main problem here.  WDYT?
>
> I think that's a very good idea.

Great! Here's my attempt:

[Message part 2 (text/html, inline)]

[0001-Document-type-error-property-for-customization-types.patch (text/x-patch, attachment)]

Message #23 received at 23975 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: rswgnu <at> gmail.com, 23975 <at> debbugs.gnu.org, Robert Weiner <rsw <at> gnu.org>
Subject: Re: bug#23975: 25.0.94: defcustom error message is wrong when :type
 field has a :match attribute
Date: Fri, 04 Sep 2020 14:51:34 +0200

Mauro Aranda <maurooaranda <at> gmail.com> writes:

> Great! Here's my attempt:

Thanks; applied to Emacs 28.

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.