Re: completing-read-multiple in Emacs 21.x

[Kevin Rodgers]

Sebastian Luque wrote:
> I'm having problems with completing-read-multiple.  e.g.:
>
> (completing-read-multiple
>  "prompt: "
>  '(("str1" 1) ("str2" 2)) nil t "str1,str2" nil "str1,str2")
>
> When called, clearing the initial entry, and typing "s [TAB]" gives "[No
> match]" in the minibuffer of Emacs 21.x.  This doesn't occur in Emacs
> 22.0.50.1.

Well, I guess there's a bug in Emacs 21 that's fixed in Emacs 22.

> In both versions though, the DEF argument (last one in the
> expression above) seems to be completely ignored, as '("")' is 
returned in
> the 22.0.50.1 version, and "[No match]" is displayed in the minibuffer of
> Emacs 21.x.  Can somebody please point out what is going on or suggest an
> alternative to reading multiple strings from a single minibuffer prompt?

That seems to be a bug in both versions, due to the fact that
completing-read-multiple assumes the result from calling
read-from-minibuffer will always be DEF and never "".  But
read-from-minibuffer's doc string warns:

| Sixth arg DEFAULT-VALUE is the default value.  If non-nil, it is available
|   for history commands; but, unless READ is non-nil, 
`read-from-minibuffer'
|   does NOT return DEFAULT-VALUE if the user enters empty input!  It 
returns
|   the empty string.

(And of course READ is nil in this case.)  That bug can be fixed with
this patch (differences due to re-indentation are suppressed):

*** lisp/emacs-lisp/crm.el~     2006-04-22 06:42:06.875000000 -0600
--- lisp/emacs-lisp/crm.el      2006-05-04 11:34:33.606827000 -0600
***************
*** 592,598 ****
  See the documentation for `completing-read' for details on the arguments:
  PROMPT, TABLE, PREDICATE, REQUIRE-MATCH, INITIAL-INPUT, HIST, DEF, and
  INHERIT-INPUT-METHOD."
!   (let ((minibuffer-completion-table (function crm-collection-fn))
        (minibuffer-completion-predicate predicate)
        ;; see completing_read in src/minibuf.c
        (minibuffer-completion-confirm
--- 592,598 ----
  See the documentation for `completing-read' for details on the arguments:
  PROMPT, TABLE, PREDICATE, REQUIRE-MATCH, INITIAL-INPUT, HIST, DEF, and
  INHERIT-INPUT-METHOD."
!   (let* ((minibuffer-completion-table (function crm-collection-fn))
         (minibuffer-completion-predicate predicate)
         ;; see completing_read in src/minibuf.c
         (minibuffer-completion-confirm
***************
*** 606,615 ****
        crm-end-of-element
        (map (if require-match
                 crm-local-must-match-map
!              crm-local-completion-map)))
!     (split-string (read-from-minibuffer
!                  prompt initial-input map
!                  nil hist def inherit-input-method)
                  crm-separator)))

  ;; testing and debugging
--- 606,618 ----
         crm-end-of-element
         (map (if require-match
                  crm-local-must-match-map
!               crm-local-completion-map))
!        (minibuffer-string (read-from-minibuffer prompt
!                                                 initial-input map nil hist def
!                                                 inherit-input-method)))
!     (split-string (if (equal minibuffer-string "")
!                     def
!                   minibuffer-string)
                  crm-separator)))

  ;; testing and debugging

There is another minor problem in the way completing-read-multiple calls
read-from-minibuffer, namely that it passes its INITIAL-INPUT argument
through as read-from-minibuffer's INITIAL-CONTENTS argument despite this
admonition in the doc string:

| The optional second arg initial-contents is an obsolete alternative to
|   default-value.  It normally should be nil in new code, except when
|   hist is a cons.  It is discussed in more detail below.

Here are those (ugly) details:

| Fifth arg hist, if non-nil, specifies a history list and optionally
|   the initial position in the list.  It can be a symbol, which is the
|   history list variable to use, or it can be a cons cell
|   (HISTVAR . HISTPOS).  In that case, HISTVAR is the history list variable
|   to use, and HISTPOS is the initial position for use by the minibuffer
|   history commands.  For consistency, you should also specify that
|   element of the history as the value of initial-contents.  Positions
|   are counted starting from 1 at the beginning of the list.
| ...
| The remainder of this documentation string describes the
| initial-contents argument in more detail.  It is only relevant when
| studying existing code, or when hist is a cons.  If non-nil,
| initial-contents is a string to be inserted into the minibuffer before
| reading input.  Normally, point is put at the end of that string.
| However, if initial-contents is (STRING . POSITION), the initial
| input is STRING, but point is placed at _one-indexed_ position
| POSITION in the minibuffer.  Any integer value less than or equal to
| one puts point at the beginning of the string.  *Note* that this
| behavior differs from the way such arguments are used in `completing-read'
| and some related functions, which use zero-indexing for POSITION.

Since completing-read-multiple takes its own HIST argument, which like
INITIAL-INPUT is to be handled as completing-read would, I guess
completing-read-multiple should pass this to read-from-minibuffer as
INITIAL-CONTENTS:

(if (and initial-input (consp hist))
    (let ((initial-string (cond ((stringp initial-input) initial-input)
                                ((consp initial-input) (car initial-input))
                                ;; control never reaches this clause:
                                (t (error "invalid INITIAL-INPUT: %s"
                                          initial-input)))))
      (when (not (equal initial-string (nth (cdr hist) (car hist))))
        (warn "INITIAL-INPUT is not consistent with HIST.
INITIAL-INPUT: %s
HIST: %s"
              initial-input hist))
      (cond ((stringp initial-input) initial-input)
            ((consp inital-input)
             ;; convert 0-indexed string position to 1-indexed
             ;; minibuffer position:
             (cons initial-string (1+ (cdr initial-input))))
            ;; control never reaches this clause:
            (t nil)))
  ;; otherwise:
  nil)

But there's no bug manifested by the current code, and I doubt that
completing-read (which is implemented in C) does anything so rigorous,
so I'm not going to provide that change as a patch.

Thanks,
--
Kevin