bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Drew Adams]

1. Use `M-x customize-option' for each of these:

(defcustom foo 42
  "Foo..."
  :type '(restricted-sexp
          :tag "Positive integer"
          :match-alternatives ((lambda (x) (and (natnump x)  (not (zerop x)))))
          :value ignore))

(defcustom bar 42
  "Bar..."
  :type '(choice (restricted-sexp
                  :tag "Positive integer"
                  :match-alternatives ((lambda (x) (and (natnump x)  (not (zerop x)))))
                  :value ignore)))

The `:tag' has no effect for `foo' - there is no label.  I think that
according to the doc it should have the same effect as for `bar'.


2. In addition, a reasonable and helpful enhancement for `choice' would
be to not show `Value Menu' at all if there is only one choice
available (i.e., when that button/menu is a no-op).


In GNU Emacs 26.1 (build 1, x86_64-w64-mingw32)
 of 2018-05-30
Repository revision: 07f8f9bc5a51f5aa94eb099f3e15fbe0c20ea1ea
Windowing system distributor `Microsoft Corp.', version 10.0.17134
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Lars Ingebrigtsen]

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

> 1. Use `M-x customize-option' for each of these:
>
> (defcustom foo 42
>   "Foo..."
>   :type '(restricted-sexp
>           :tag "Positive integer"
>           :match-alternatives ((lambda (x) (and (natnump x)  (not (zerop x)))))
>           :value ignore))
>
> (defcustom bar 42
>   "Bar..."
>   :type '(choice (restricted-sexp
>                   :tag "Positive integer"
>                   :match-alternatives ((lambda (x) (and (natnump x)  (not (zerop x)))))
>                   :value ignore)))
>
> The `:tag' has no effect for `foo' - there is no label.  I think that
> according to the doc it should have the same effect as for `bar'.

I can confirm that this behaviour is still present in Emacs 28.  Mauro,
if you have time, could you take a look at this?

> 2. In addition, a reasonable and helpful enhancement for `choice' would
> be to not show `Value Menu' at all if there is only one choice
> available (i.e., when that button/menu is a no-op).

Makes sense.

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

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]

> Drew Adams <drew.adams@oracle.com> writes:
>
> > 1. Use `M-x customize-option' for each of these:
> >
> > (defcustom foo 42
> >   "Foo..."
> >   :type '(restricted-sexp
> >           :tag "Positive integer"
> >           :match-alternatives ((lambda (x) (and (natnump x)  (not
(zerop x)))))
> >           :value ignore))
> >
> > (defcustom bar 42
> >   "Bar..."
> >   :type '(choice (restricted-sexp
> >                   :tag "Positive integer"
> >                   :match-alternatives ((lambda (x) (and (natnump x)
 (not (zerop x)))))
> >                   :value ignore)))
> >
> > The `:tag' has no effect for `foo' - there is no label.  I think that
> > according to the doc it should have the same effect as for `bar'.
>
> I can confirm that this behaviour is still present in Emacs 28.  Mauro,
> if you have time, could you take a look at this?

Not much time until the weekend, I'm afraid.

Dropping the tag is intentional, in custom-variable-value-create:
(push (widget-create-child-and-convert
   widget type
   :format value-format
    ^^^^^^^^^^^^^^^^^^^
   :value value)
  children))

I suppose we could stop overriding the :format property, but for some
widgets overriding it might make sense.  For example, for the choice
widget, deleting the :format value-format line would create the
following:

Foo: Choice: [Value Menu] The-Tag:

Which isn't good, IMO.  Other customization types I can think of that we
should pay attention if we go with this change would be: repeat, set and
radio.

I think that those three, if we print their tag, won't give too much
valuable information about the variable.   I mean, we'd end up with
something like this:

Foo: Repeat:
[INS] [DEL] Something
[INS]

And any user may ask what does "repeat" mean.  Maybe changing the tags
to something slightly more useful is all we need, and with this change
the Custom buffer will show the customization type of the variable to
the user, which looks like a win to me.  What do you think about the
"problematic" tags?

[-- Attachment #2: Type: text/html, Size: 2604 bytes --]

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Drew Adams]

> Not much time until the weekend, I'm afraid.
>
> Dropping the tag is intentional, in custom-variable-value-create:
> (push (widget-create-child-and-convert
>   widget type
>   :format value-format
>    ^^^^^^^^^^^^^^^^^^^
>   :value value)
>  children))
>
> I suppose we could stop overriding the :format property, but for some
> widgets overriding it might make sense.  For example, for the choice
> widget, deleting the :format value-format line would create the
> following:
>
> Foo: Choice: [Value Menu] The-Tag:
>
> Which isn't good, IMO.  Other customization types I can think of that we
> should pay attention if we go with this change would be: repeat, set and
> radio.
>
> I think that those three, if we print their tag, won't give too much
> valuable information about the variable.   I mean, we'd end up with
> something like this:
>
> Foo: Repeat:
> [INS] [DEL] Something
> [INS]
>
> And any user may ask what does "repeat" mean.  Maybe changing the tags
> to something slightly more useful is all we need, and with this change
> the Custom buffer will show the customization type of the variable to
> the user, which looks like a win to me.  What do you think about the
> "problematic" tags?

Thanks for looking into this Mauro.

I'd suggest handling this in two stages:

1. Take care of what is clearly, or pretty clearly,
   straightforward.
2. Think about how to handle other cases.
____

But in general, for defcustom I think I'm in favor of
allowing the realization of a :tag, regardless of
whether using it might be problematic sometimes.

After all, using :tag is optional.  If the result
isn't helpful, someone won't use it.

But I guess you're asking about default tags?  What
happens if there's no default :tag for some widget
(such as `repeat') but when you use the widget you
provide a :tag?  Would that be possible?  IOW, maybe
a :tag for `repeat' isn't useful by default, but
maybe adding a :tag when you use some `repeat' could
be useful.

(For the problematic cases, maybe the tag text should be
shown without the trailing colon?  Maybe it depends on
where the tag is placed and how long the string is.)

Anyway, for now at least, #1 would be great.  Thx.

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Mauro Aranda]

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

> Thanks for looking into this Mauro.
>
> I'd suggest handling this in two stages:
>
> 1. Take care of what is clearly, or pretty clearly,
>    straightforward.
> 2. Think about how to handle other cases.
> ____
>
> But in general, for defcustom I think I'm in favor of
> allowing the realization of a :tag, regardless of
> whether using it might be problematic sometimes.

I agree with you.  It's just that since it's an intentional behavior (or
at least looks like that), I wanted to make sure we don't miss any
potentially annoying side effects.

> But I guess you're asking about default tags?  What
> happens if there's no default :tag for some widget
> (such as `repeat') but when you use the widget you
> provide a :tag?  Would that be possible?  IOW, maybe
> a :tag for `repeat' isn't useful by default, but
> maybe adding a :tag when you use some `repeat' could
> be useful.

Indeed, I was worried about the default tags.  But then I realized that
we already show the "Repeat" tag, when the repeat is part of a choice
type, so it looks like my worries are void.  Same goes for the choice
widget inside any other grouping widget.

> (For the problematic cases, maybe the tag text should be
> shown without the trailing colon?  Maybe it depends on
> where the tag is placed and how long the string is.)

I think it's better to keep the colon.

> Anyway, for now at least, #1 would be great.  Thx.

AFAICS, we should be able to do #1 without trouble, by removing the
:format value-format line, and accounting for the space after the tag
when creating the item.

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Drew Adams]

I have confidence that whatever you decide to do will be an improvement.  Thanks for working on this.

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> I suppose we could stop overriding the :format property, but for some
> widgets overriding it might make sense.  For example, for the choice
> widget, deleting the :format value-format line would create the
> following:
>
> Foo: Choice: [Value Menu] The-Tag:
>
> Which isn't good, IMO.

Yeah, that doesn't look optimal...

> Other customization types I can think of that we should pay attention
> if we go with this change would be: repeat, set and radio.
>
> I think that those three, if we print their tag, won't give too much
> valuable information about the variable.   I mean, we'd end up with
> something like this:
>
> Foo: Repeat:
> [INS] [DEL] Something
> [INS]
>
> And any user may ask what does "repeat" mean.  Maybe changing the tags
> to something slightly more useful is all we need, and with this change
> the Custom buffer will show the customization type of the variable to
> the user, which looks like a win to me.

Sounds good to me, if I understand you correctly (which I may very well
not do). 

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

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 1773 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> I suppose we could stop overriding the :format property, but for some
>> widgets overriding it might make sense.  For example, for the choice
>> widget, deleting the :format value-format line would create the
>> following:
>>
>> Foo: Choice: [Value Menu] The-Tag:
>>
>> Which isn't good, IMO.
>
> Yeah, that doesn't look optimal...

Earlier today I sent a message saying we already show it that way when
the choice widget is part of another grouping widget.  And I can't
figure out a nice way to show it, while being backward compatible...

So I suggest the attached patch, and if the
Choice: [Value Menu] text turns out to be really annoying, we can
explore some other way of fixing it.  (If you want, I can send a
tentative untested example of how a less backward compatible change would
look like).

>> Other customization types I can think of that we should pay attention
>> if we go with this change would be: repeat, set and radio.
>>
>> I think that those three, if we print their tag, won't give too much
>> valuable information about the variable.   I mean, we'd end up with
>> something like this:
>>
>> Foo: Repeat:
>> [INS] [DEL] Something
>> [INS]
>>
>> And any user may ask what does "repeat" mean.  Maybe changing the tags
>> to something slightly more useful is all we need, and with this change
>> the Custom buffer will show the customization type of the variable to
>> the user, which looks like a win to me.
>
> Sounds good to me, if I understand you correctly (which I may very well
> not do). 

Similarly to the choice widget, we already show "Repeat" and such things
in other situations, so I'd say my initial worries should not be taken
into account.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Bug35133 patch --]
[-- Type: text/x-patch, Size: 1883 bytes --]

From f0d69b47349fd23fd06a8f1c5f54ca3bfcfc6522 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Fri, 11 Dec 2020 13:07:21 -0300
Subject: [PATCH] Stop dropping the tag when creating the custom-variable
 widget

* lisp/cus-edit.el (custom-variable-value-create): Obey the specified
tag format when creating the variable tag, but stop dropping the tag
format for the variable's type widget, since the tag can be used to
give useful information to the user about the variable.  (Bug#35133)
---
 lisp/cus-edit.el | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index 3a36cb0237..041f13b420 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -2734,11 +2734,15 @@ custom-variable-value-create
 		 buttons)
 	   (insert " ")
 	   (let* ((format (widget-get type :format))
-		  tag-format value-format)
-	     (unless (string-match ":" format)
+                  tag-format)
+             ;; We used to drop the widget tag when creating TYPE, passing
+             ;; everything after the colon (including whitespace characters
+             ;; after it) as the :format for TYPE.  We don't drop the tag
+             ;; anymore, but we should keep an immediate whitespace character,
+             ;; if present, and it's easier to do it here.
+             (unless (string-match ":\\s-?" format)
 	       (error "Bad format"))
 	     (setq tag-format (substring format 0 (match-end 0)))
-	     (setq value-format (substring format (match-end 0)))
 	     (push (widget-create-child-and-convert
 		    widget 'item
 		    :format tag-format
@@ -2753,7 +2757,6 @@ custom-variable-value-create
 		   buttons)
 	     (push (widget-create-child-and-convert
 		    widget type
-		    :format value-format
 		    :value value)
 		   children))))
     (unless (eq custom-buffer-style 'tree)
-- 
2.29.2

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Similarly to the choice widget, we already show "Repeat" and such things
> in other situations, so I'd say my initial worries should not be taken
> into account.

[...]

> * lisp/cus-edit.el (custom-variable-value-create): Obey the specified
> tag format when creating the variable tag, but stop dropping the tag
> format for the variable's type widget, since the tag can be used to
> give useful information to the user about the variable.  (Bug#35133)

I've not tested it, but the patch makes sense to me -- go ahead and push.

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

bug#35133: 26.1; 1) `:tag' for `restricted-sexp' (not in a choice, set, etc.), 2) Remove `Value Menu' if a no-op

[Mauro Aranda]

tags 35133 fixed
close 35133 28.1
quit

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Similarly to the choice widget, we already show "Repeat" and such things
>> in other situations, so I'd say my initial worries should not be taken
>> into account.
>
> [...]
>
>> * lisp/cus-edit.el (custom-variable-value-create): Obey the specified
>> tag format when creating the variable tag, but stop dropping the tag
>> format for the variable's type widget, since the tag can be used to
>> give useful information to the user about the variable.  (Bug#35133)
>
> I've not tested it, but the patch makes sense to me -- go ahead and push.

Thanks.  Pushed, and closing.