Re: Recent read-face patch breaks `M-x customize-face'

["Γιώργος Κεραμίδας" <gkeramidas@gmail.com>]

On Sat, 06 Apr 2013 10:02:52 -0400, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>> It would be safer to simply turn any non-list value of `default' into
>>> a one-element list.
>> Makes sense.  Yet the problem here is that customize-face calls
>> read-face-name with DEFAULT having the bogus value "all faces".
>> So customize-face should still be fixed.
>
> Just for the record: the reason for this "all faces" argument is so that
> M-x customize-face prompts with "Customize face (default `all faces'):"

Ah, that's where that message came from!  I see now that no message
about (default `xxx') is displayed at the M-x prompt.  Maybe we have to
change read-face-name a bit.

Right now the doc string of read-face-name says things in two places
that when combined together seem to imply that what customize-face
expects is ok:

    (read-face-name PROMPT &optional DEFAULT MULTIPLE)

    Read one or more face names, defaulting to the face(s) at point.
    PROMPT should be a prompt string; it should not end in a space or
    a colon.

  : The optional argument DEFAULT specifies the default face name(s)
  : to return if the user just types RET.  If its value is non-nil,
  : it should be a list of face names (symbols or strings); in that case,
  : the default return value is the `car' of DEFAULT (if the argument
  : MULTIPLE is non-nil), or DEFAULT (if MULTIPLE is nil).  See below
  : for the meaning of MULTIPLE.

    If DEFAULT is nil, the list of default face names is taken from
    the symbol at point and the `read-face-name' property of the text at point,
    or, if that is nil, from the `face' property of the text at point.

    This function uses `completing-read-multiple' with "[ \t]*,[ \t]*"
    as the separator regexp.  Thus, the user may enter multiple face
  : names, separated by commas.  The optional argument MULTIPLE
  : specifies the form of the return value.  If MULTIPLE is non-nil,
  : return a list of face names; if the user entered just one face
  : name, the return value would be a list of one face name.
  : Otherwise, return a single face name; if the user entered more
  : than one face name, return only the first one.

If we read together the two marked parts they seem to translate to what
customize-face expects, that when DEFAULT is nil and, at the same time,
MULTIPLE is non-nil, a list of all faces is returned.

What `read-face-name' does when DEFAULT is non-nil makes, of course,
sense: display the current value of DEFAULT in the prompt.  But maybe we
should change it to also work correctly if MULTIPLE is non-nil and
DEFAULT is not the name of a symbol.

What was failing was this part of `read-face-name':

  : ;; If we only want one, and the default is more than one,
  : ;; discard the unwanted ones now.
  : (if (and default (not multiple))
  :     (setq default (list (car default))))
  :
  : (if default
  :     (setq default (mapconcat (lambda (f)
  :                                (if (symbolp f) (symbol-name f) f))
  :                              default ", ")))

Maybe we just need to account for the case of:

    (if (and multiple default (not (symbolp default)) (not (consp default)))
      just-use-default-value-as-prompt)

Because what happened here was that `default' was passed as "all faces",
and mapconcat happily tried to filter the string through the lambda
function, resulting in a list of character values, resulting in a call
like this:

  (mapconcat (lambda (x) x)
    (list 97 108 108 32 102 97 99 101 115)
    ", ")

Which of course throws a traceback, when mapconcat tries to look at the
character value `97' as a sequence to concatenate it with the rest.