bug#33566: 26; `group' :type for defcustom

bug#33566: 26; `group' :type for defcustom

[Drew Adams]

It seems like defcustom composite :type `group' does not work.

See https://emacs.stackexchange.com/q/46357/105.

emacs -Q

(defcustom foo '(1 2 3)
  "..."
  :type '(group integer integer integer)
  :group 'emacs)

M-x customize-option foo

Shows this message in *Messages*:

custom-variable-value-create: Bad format

And buffer *Customize Option: Foo* is not complete.  I see only this:

------------8<----------------

Custom settings cannot be saved; maybe you started Emacs with '-q'.
For help using this buffer, see Easy Customization in the Emacs manual.

                                          Search 

Operate on all settings in this buffer:
 Revert...   Apply  

Hide 
------------8<----------------

The doc says that :type `group' should be very similar to :type `list',
but without tags.

See also this bug:
https://lists.gnu.org/archive/html/emacs-pretest-bug/2007-06/msg00067.html

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.16299
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

bug#33566: 26; `group' :type for defcustom

[Phil Sainty]

This appears to be the same issue as bug #31309

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31309


At present in wid-edit.el we see:

(define-widget 'group 'default
  [...]
  :format "%v"

And that format of "%v" is triggering the error.

In #31309 I noted that:

> the parent `editable-field' widget says:
> "Note: In an ‘editable-field’ widget, the ‘%v’ escape must be preceded
> by some other text in the ‘:format’ string (if specified)."

And so I copied the :format "%{%t%}: %v" used by the 'string type.

If we make that same change here:

(define-widget 'group 'default
  [...]
  :format "%{%t%}: %v"

Then the example code from this new bug works.


Possibly there are other such bugs here as well?

8 matches for ":format "%v"" in buffer: wid-edit.el
    594:  :type '(repeat (cons :format "%v"
    597:			       (string :format "%v")))))
   1870:  :format "%v"
   2214:  :format "%v"
   2392:  :format "%v"
   2749:  :format "%v"
   2905:  :format "%v"
   3565:				(cons :format "%v"



-Phil

bug#33566: 26; `group' :type for defcustom

[Drew Adams]

> This appears to be the same issue as bug #31309
> 
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__debbugs.gnu.org_cgi_bugreport.cgi-3Fbug-
> 3D31309&d=DwIDaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=kI3P6l
> jGv6CTHIKju0jqInF6AOwMCYRDQUmqX22rJ98&m=q_aD3h6_uQ5XqacM7h8h0yyr4bJrYed
> Kx_CcGGTUXRU&s=BlmxwvxgNdZTq6QqgdDRglwd0qa8LpG-SLOp13SLoV4&e=
> 
> 
> At present in wid-edit.el we see:
> 
> (define-widget 'group 'default
>   [...]
>   :format "%v"
> 
> And that format of "%v" is triggering the error.
> 
> In #31309 I noted that:
> 
> > the parent `editable-field' widget says:
> > "Note: In an ‘editable-field’ widget, the ‘%v’ escape must be
> preceded
> > by some other text in the ‘:format’ string (if specified)."
> 
> And so I copied the :format "%{%t%}: %v" used by the 'string type.
> 
> If we make that same change here:
> 
> (define-widget 'group 'default
>   [...]
>   :format "%{%t%}: %v"
> 
> Then the example code from this new bug works.
> 
> 
> Possibly there are other such bugs here as well?
> 
> 8 matches for ":format "%v"" in buffer: wid-edit.el
>     594:  :type '(repeat (cons :format "%v"
>     597:			       (string :format "%v")))))
>    1870:  :format "%v"
>    2214:  :format "%v"
>    2392:  :format "%v"
>    2749:  :format "%v"
>    2905:  :format "%v"
>    3565:				(cons :format "%v"

Great.  Good catch and follow-up.

A couple things:

This (e.g. the recipe in bug #33566) has apparently been broken since at least Emacs 20.  But in Emacs 20 the message in *Messages* is just "Bad format".

But :group is supposed to not show any :tag.  So I think this is what we should use:

(define-widget 'group 'default
  "A widget which groups other widgets inside."
  :convert-widget 'widget-types-convert-widget
  :copy 'widget-types-copy
  :format ":\n%v"  ; <================
  :value-create 'widget-group-value-create
  :value-get 'widget-editable-list-value-get
  :default-get 'widget-group-default-get
  :validate 'widget-children-validate
  :match 'widget-group-match
  :match-inline 'widget-group-match-inline)

There doesn't seem to be any way to eliminate the `:' altogether, though.

I think the doc needs to be corrected in (widget) Editable Text Fields.  This is not correct/sufficient;

  *Warning:* In an ‘editable-field’ widget, the ‘%v’
  escape must be preceded by some other text in the
  ‘:format’ string (if specified).

It's not sufficient that just any old text precede
the `%v'.  There must be a colon, AFAICT.

It should instead say something like this:

  *Warning:* In an ‘editable-field’ widget, the ‘%v’
  escape must be preceded by a colon (`:') in the
  ‘:format’ string (if specified).  No tag name need
  precede the colon, but the colon must be present.
  (Additional text can follow the colon and precede
  the ‘%v’.)

(Perhaps more testing or checking of just what can
precede the %v needs to be done.)

I'd also drop the "*Warning:*", personally, and
instead say that this is necessary or you get a
bad format message (or some such).

bug#33566: [PATCH] RE: bug#33566: 26; `group' :type for defcustom

[Drew Adams]

I think the patch below fixes the code bug.

Someone else can consider fixing the doc bug:

> I think the doc needs to be corrected in (widget)
> Editable Text Fields.  This is not correct/sufficient;
> 
>   *Warning:* In an ‘editable-field’ widget, the ‘%v’
>   escape must be preceded by some other text in the
>   ‘:format’ string (if specified).
> 
> It's not sufficient that just any old text precede
> the `%v'.  There must be a colon, AFAICT.
> 
> It should instead say something like this:
> 
>   *Warning:* In an ‘editable-field’ widget, the `%v'
>   escape must be preceded by a colon (`:') in the
>   `:format' string (if specified).  No tag name need
>   precede the colon, but the colon must be present.
>   (Additional text can follow the colon and precede
>   the `%v'.)
...
> I'd also drop the "*Warning:*", personally, and
> instead say that this is necessary or you get a
> bad format message (or some such).

---

diff -u wid-edit.el wid-edit-2018-12-03a-patched.el
--- wid-edit.el	2018-12-03 12:37:30.626331700 -0800
+++ wid-edit-2018-12-03a-patched.el	2018-12-03 12:43:05.440329000 -0800
@@ -2746,7 +2746,7 @@
   "A widget which groups other widgets inside."
   :convert-widget 'widget-types-convert-widget
   :copy 'widget-types-copy
-  :format "%v"
+  :format ":\n%v"
   :value-create 'widget-group-value-create
   :value-get 'widget-editable-list-value-get
   :default-get 'widget-group-default-get

bug#33566: [PATCH] RE: bug#33566: 26; `group' :type for defcustom

[Drew Adams]

ping.  Could someone please consider committing
this patch.  Thx.

> I think the patch below fixes the code bug.

> diff -u wid-edit.el wid-edit-2018-12-03a-patched.el
> --- wid-edit.el	2018-12-03 12:37:30.626331700 -0800
> +++ wid-edit-2018-12-03a-patched.el	2018-12-03 12:43:05.440329000 -0800
> @@ -2746,7 +2746,7 @@
>    "A widget which groups other widgets inside."
>    :convert-widget 'widget-types-convert-widget
>    :copy 'widget-types-copy
> -  :format "%v"
> +  :format ":\n%v"
>    :value-create 'widget-group-value-create
>    :value-get 'widget-editable-list-value-get
>    :default-get 'widget-group-default-get

bug#33566: [PATCH] RE: bug#33566: 26; `group' :type for defcustom

[Eli Zaretskii]

> Date: Sat, 22 Dec 2018 15:35:40 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> 
> ping.  Could someone please consider committing
> this patch.  Thx.

Sorry for the delay.  Installed on the emacs-26 branch.