tags 44579 patch
quit

Leo Vivier <zaeph@zaeph.net> writes:

> Hi there,
>
> There seems to be a problem in `defcustom' forms with the way the
> `choice' widget handles `:inline t'.
>
> I’ve written an .el file to walk you through it: I’ve documented the
> bug, the explanation, and a tentative solution.
>

[...]

> ;; >    When the element-type is a ‘choice’, you use ‘:inline’ not in the
> ;; > ‘choice’ itself, but in (some of) the alternatives of the ‘choice’.
For
> ;; > example, to match a list which must start with a file name, followed
> ;; > either by the symbol ‘t’ or two strings, use this customization type:
> ;; >
> ;; >      (list file
> ;; >            (choice (const t)
> ;; >                    (list :inline t string string)))
> ;; >
> ;; > If the user chooses the first alternative in the choice, then the
> ;; > overall list has two elements and the second element is ‘t’.  If the
> ;; > user chooses the second alternative, then the overall list has three
> ;; > elements and the second and third must be strings.
>
> ;; The first option in ‘choice’ works.
> (defcustom zp/testing '("foo" t)
>   "Testing variable."
>   :type
>   '(list file
>          (choice (const t)
>                  (list :inline t
>                    string
>                    string))))
>
> (customize-variable 'zp/testing)
> ;; => The form is recognised.
>
> ;; The second option in ‘choice’ doesn’t work.
> (defcustom zp/testing '("foo" "bar" "baz")
>   "Testing variable."
>   :type
>   '(list file
>          (choice (const t)
>                  (list :inline t
>                    string
>                    string))))
>
> (customize-variable 'zp/testing)
> ;; => The form is *not* recognised.

Confirmed.

> ;;; SUGGESTED FIX
>
> ;; ‘choice’ needs to be made aware of the ":inline t" in its options.
> ;;
> ;; Since ‘choice’ is intended to receed into the background once the
> ;; appropriate option has been pattern-matched, it doesn’t make sense to
have
> ;; it carry the ":inline t".  Instead, it should respect the ":inline t"
that
> ;; is present in its option when said option is matched.

Yes, something like that.  This bug happens because the choice widget is
unable to tell to widget-match-inline that it wants to try to match more
than one member of a list, when one of its choices is inline.  So
widget-match-inline only passes it one element, in this case a string,
and one string won't match a list of two strings.

So the choice widget needs to be able to tell widget-match-inline about
that.  To avoid a large impact of tweaking the code to fix this, I made
a change to affect only choice widgets with inline choices, which are
the ones that suffer exhibit this bug.

The patch to wid-edit.el is a little larger, because once the choice
widget can match inline values, then it has to be able to create them.

I added tests for both matching choice widgets and creating the choice
widgets as a part of other grouping widgets.  In current master, the
following tests should fail, exposing the bug:
widget-test-choice-match-all-inline
widget-test-choice-match-some-inline

And without the changes to the create functions, the following tests
would fail:
widget-test-repeat-can-handle-inlinable-choice
widget-test-list-can-handle-inlinable-choice
widget-test-option-can-handle-inlinable-choice