unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#44579: Unintended behaviour with defcustom’s ‘choice’ widgets and ":inline t" & Wrong documentation on "(elisp) Splicing into Lists"
@ 2020-11-11 15:28 Leo Vivier
  2020-11-16 23:48 ` Mauro Aranda
  0 siblings, 1 reply; 5+ messages in thread
From: Leo Vivier @ 2020-11-11 15:28 UTC (permalink / raw)
  To: 44579

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

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.

I recommended downloading the attachment and eval’ing the forms as they
appear.  Make sure that you use C-M-x with the `defcustom' forms, lest
their STANDARD not be reset.

HTH,

-- 
Leo Vivier
Freelance Software Engineer
Website: www.leovivier.com | Blog: www.zaeph.net

[-- Attachment #2: defcustom-inline-debug.el --]
[-- Type: text/emacs-lisp, Size: 13476 bytes --]

;;; DESCRIPTION

;; Incriminated section of the manual:
(info "(elisp) Splicing into Lists")

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

;; Proof that ‘choice’ implies a list in this context by enclosing the result
;; of the ‘choice’ in a list in STANDARD.
(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 recognised.



;;; DEBUGGING

;; Adding another ":inline t" for `choice' allows us to make progress.
(defcustom zp/testing '("foo" "bar" "baz")
  "Testing variable."
  :type
  '(list file
         (choice :inline t            ; <---
          (const t)
          (list :inline t
            string
            string))))

(customize-variable 'zp/testing)
;; => The form is recognised.

;; …But this makes the first option not recognised t as a legitimate value.
(defcustom zp/testing '("foo" t)
  "Testing variable."
  :type
  '(list file
         (choice :inline t            ; <---
          (const t)
          (list :inline t
            string
            string))))

(customize-variable 'zp/testing)
;; => The form is recognised, but t is not recognised as a valid value.
;; However, manually selecting t in the "Value menu" produces a form which you
;; can apply, producing exactly the same form as STANDARD.  Closing and
;; reopening the customise window after applying the value produces the same
;; invalid value, as expected.

;; Enclosing t in a list resolves the issue.
(defcustom zp/testing '("foo" "bar" "baz")
  "Testing variable."
  :type
  '(list file
         (choice :inline t
           (list :inline t              ; <---
             (const t))
           (list :inline t
             string
             string))))

(customize-variable 'zp/testing)
;; => The form is recognised, but at what cost?
;; Also, it is slightly misleading to present the first choice as a "List" in
;; the menu, considering that the list is exploded into its single element.

;; …Let’s dive deeper, shall we?



;;; EXPLANATION (for the courageous)

;; The issue can be more easily grokked if we consider a close alternative to
;; the ‘choice’ widget: ‘set’.  Instead of the XOR presented in the previous
;; choices, we’re going to turn it into an OR and enclose it into a list.
(defcustom zp/testing '("foo" (t "bar" "baz")) ; <---
  "Testing variable."
  :type
  '(list file
         (set                       ;No ":inline t" to keep the the list as is
          (const t)
          (list :inline t
            string
            string))))

(customize-variable 'zp/testing)
;; => The form is recognised.
;; As we can see, the ‘set’ widget always implies a list, which is why all the
;; options are enclosed in a list.

;; Now, let’s explode that list by adding ":inline t" to the ‘set’ widget:
(defcustom zp/testing '("foo" t "bar" "baz") ; <---
  "Testing variable."
  :type
  '(list file
         (set :inline t                 ; <---
           (const t)
           (list :inline t
             string
             string))))

(customize-variable 'zp/testing)
;; => The form is recognised.

;; If we focus on the second option in ‘set’, we can see that ‘set’ receives
;; the constant t alongside two strings which are the elements of an exploded
;; list.  Therefore, ‘set’ is handed three elements which it packages into
;; a list.  This list is then exploded via ":inline t".
;; (Please note that my usage of "handed over" is used to describe what is
;; *seemingly* happening, which is not how the code actually implements it, as
;; we’ll see later.)
;;
;; The difference between the ‘set’ widget and the ‘choice’ widget is that the
;; later does not always imply a list.
;;
;; Let’s return to the previous non-functioning examples with ‘choice’ and
;; ":inline t".

;; Scenario 1: 2nd option for ‘choice’
(defcustom zp/testing '("foo" "bar" "baz") ; <---
  "Testing variable."
  :type
  '(list file
         (choice :inline t
           (const t)
           (list :inline t         ; <--- MATCH
             string
             string))))

(customize-variable 'zp/testing)
;; => The form is recognised.
;; Here, we have a very similar scenario to the one which was just described
;; with ‘set’: ‘choice’ is handed an exploded list which it packages into
;; a list.  This list is then exploded via ":inline t".

;; Scenario 2: 1st option for ‘choice’ *without* a wrapping list
(defcustom zp/testing '("foo" t)
  "Testing variable."
  :type
  '(list file
         (choice :inline t
           (const t)               ; <--- MATCH?
           (list :inline t
             string
             string))))

(customize-variable 'zp/testing)
;; => The form is recognised, but t is not recognised as a valid value.
;; In this scenario, ‘choice’ receives a single element, t, and is instructed
;; to explode it via ":inline t".  This results in an existential panic
;; (« Sacrebleu, ceci n’est pas une liste ?! ») which ultimately causes
;; ‘customize’ to not parse the form properly.

;; Scenario 3: 1st option for ‘choice’ *with* a wrapping list
(defcustom zp/testing '("foo" t)
  "Testing variable."
  :type
  '(list file
         (choice :inline t
           (list :inline t         ;Wrapping list
             (const t))            ; <--- MATCH
           (list :inline t
             string
             string))))

(customize-variable 'zp/testing)
;; => The form is recognised, but at what cost?

;; If we pause here for a moment, this is the reason why we need to revise the
;; "handed over" model I described above.  If ‘choice’ and ‘set’ exploded
;; their elements in the order I detailed, elements shouldn’t be discriminated
;; upon their provenance, i.e., whether they were atomic elements or if they
;; came from an exploded list.

;; Let’s essentialise the form a little bit to clarify my point:
;; (All the examples below produce well-formed results for ‘customize’.)
(defcustom zp/testing t            ;Atomic element
  "Testing variable."
  :type
  '(choice
    (list :inline t
      (const t))
    (const t)))                    ; <--- MATCH

(customize-variable 'zp/testing)
;; Based on the order of the options, if the 1st option had been exploded
;; before considering STANDARD, it should have matched.  Instead, it’s the 2nd
;; option which matches.

(defcustom zp/testing '(t)         ;List with an atomic element
  "Testing variable."
  :type
  '(choice
    (list :inline t                ; <--- MATCH
      (const t))
    (const t)))

(customize-variable 'zp/testing)
;; When STANDARD is a list, the 1st option matches in spite of the exploded
;; list, which goes against what is described in the manual.

;; My interpretation is that ‘choice’ was not designed with ":inline t" in
;; mind, or at least not as ‘set’ was.  I haven’t investigated how the parsing
;; is done internally, but I assume that ‘choice’ fails to understand ":inline
;; t" in composite widgets.

;; Just to be clear, ":inline t" implies that we want the elements in the
;; composite widget to be merged inside the super-element, i.e., the one which
;; *includes* the composite widget with ":inline t".

;; Let’s refer back to the examples with ‘set’ to illustrate this:
(defcustom zp/testing '("foo" (t "bar" "baz")) ; <---
  "Testing variable."
  :type
  '(list file
         (set              ;No ":inline t" to keep the the list as is
          (const t)
          (list :inline t
            string
            string))))

(customize-variable 'zp/testing)
;; => The form is recognised.

(defcustom zp/testing '("foo" t "bar" "baz") ; <---
  "Testing variable."
  :type
  '(list file
         (set :inline t    ;Now with ":inline t"
          (const t)
          (list :inline t
            string
            string))))

(customize-variable 'zp/testing)
;; => The form is recognised.

;; Now, the fundamental problem with ‘choice’---and mind the quotes, this
;; isn’t Philosophy 101---is that it is a composite widget which must
;; *sometimes* return non-composite widgets:
;;
;; 1. (choice symbol symbol)
;;    always returns a non-composite widget
;;
;; 2. (choice (list symbol) (list symbol))
;;    always returns a composite widget
;;
;; 3. (choice (list symbol) symbol)
;;    *sometimes* returns a non-composite widget
;;    *sometimes* returns a composite widget
;;
;; Because of this, the current implementation of ‘choice’ always tries to
;; return a single well-formed widget.  When we use ":inline t" on a composite
;; widget, we explicity tell that widget that we don’t care about its opinion
;; and we explode it.  If it only had one element, Mazel-fucking-tov, we’ve
;; got a well-formed non-composite widget!  But if it had more than one
;; element, all we’re left with is a a group of elements which aren’t in
;; a list, but which would have to be coerced into one for ‘choice’ to
;; consider them as a well-formed option.
;;
;; So, since all widgets are created equals, rather than discriminating upon
;; well-formedness---society abhors originals, after all---we coerce this
;; rebelious group into a list to set the world aright.
;;
;; But there is a way, and I’ll explain it in the next section.



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

;; Let’s walk through an example to clarify:
(defcustom zp/testing t    ;Atomic element
  "Testing variable."
  :type
  '(choice
    (list :inline t        ;Option 1
      (const t))
    (const t)))            ;Option 2

(customize-variable 'zp/testing)
;; (For those of you following at home, this was the first essentialised
;; example.  If you didn’t read the previous section and you’re feeling left
;; out, that’s what you get for skipping my lecture, you ingrate.)

;; With the *current implementation*, Option 2 is matched.  Option 1 is
;; exploded as instructed, but ‘choice’ coerces it into a list in order to
;; return it as a well-formed widget.
;;
;; Instead, when ‘choice’ pattern-matches an option which has ":inline t" in
;; it (notwithstanding those which might be nested if the option happens to be
;; a composite widget), it should *distribute* that ":inline t" to the
;; ‘choice’ widget itself so as to return the rebellious group of elements
;; (that’s another reference you might have missed from the previous section;
;; sucks to be you, eh?).
;;
;; So, in short: when ‘choice’ has an option with ":inline t", explode the
;; option for pattern-matching, and if it matches, distribute the ":inline t"
;; to the ‘choice’ widget.
;;
;;                                     ***
;;
;; Now, I know what you’re thinking: why spend so long documenting a problem
;; rather than *actually* trying to fix it?  Well, firstly, congratulations,
;; you’ve described academia, but I also happen to be an English major, and
;; I’ll jump on every occasion I get to offset my technical-illiterateness
;; with my actual-literateness. Secondly I couldn’t believe the doc of Emacs
;; to be any less than perfect: claiming otherwise would have been hubris, and
;; I had no idea what pandemonium awaited me if I dared to refactor the
;; scriptures.
;;
;; Thank you for listening to my TED Talk.^A^K
;; Remember that the real treasure is the friends we made along the way.^A^K
;;
;; …Thank you.

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-24 11:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-11 15:28 bug#44579: Unintended behaviour with defcustom’s ‘choice’ widgets and ":inline t" & Wrong documentation on "(elisp) Splicing into Lists" Leo Vivier
2020-11-16 23:48 ` Mauro Aranda
2020-11-17 17:56   ` Leo Vivier
2020-11-24  6:36   ` Lars Ingebrigtsen
2020-11-24 11:33     ` Mauro Aranda

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).