GNU bug report logs - #1085
23.0.60; all-completions, try-completion inconsistent: Info-read-node-name-1

Previous Next

Full log

Message #20 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: <1085 <at> debbugs.gnu.org>, <emacs-pretest-bug <at> gnu.org>
Subject: RE: bug#1085: 23.0.60;all-completions, try-completion inconsistent: Info-read-node-name-1
Date: Sun, 5 Oct 2008 15:53:50 -0700

> From: Drew Adams Sent: Saturday, October 04, 2008 4:18 PM
> > From: Drew Adams Sent: Saturday, October 04, 2008 3:58 PM
> >
> > emacs -Q
> > C-h i
> >  
> > M-: (try-completion "(el" 'Info-read-node-name-1)
> >  
> > It returns "(elisp)", meaning that this is the common prefix of all
> > completions of "(el". [This is reasonable, and it satisfies the
> > requirement that "(el" is a prefix of "(elisp)".]
> >  
> > M-: (all-completions "(el" 'Info-read-node-name-1)
> >  
> > It returns ("elisp"), meaning that the only valid 
> completion of "(el"
> > is "elisp". But "elisp" does not have the common prefix "(elisp)" as
> > determined by `try-completion', and "elisp" does not even have the
> > input "(el" as a prefix. This is inconsistent.  `all-completions'
> > should return ("(elisp)") in this case.
> >  
> > Lisp code needs to be able to depend on the fact that the valid
> > completions returned by `all-completions' have the common prefix
> > that is returned by `try-completion' (which must in turn have the
> > input as its prefix).
> > 
> > And each of the completions returned by `all-completions' must
> > also satisfy `test-completion'.  In particular,
> > (test-completion STRG (all-completions strg TABLE)) must always
> > return t, for all STRG and TABLE. In this case, for STRG = "(el" and
> > TABLE = `Info-read-node-name-1', it returns nil.
> > 
> > One should be able to use `all-completions' to construct a cons
> > completion table that is equivalent to the original TABLE argument,
> > regardless of how TABLE is defined (e.g. function, obarray).  That
> > is, when used with the same inputs it should have the same effect,
> > in particular for `try-completion', `all-completions', and
> > `test-completion'.
> > 
> > I don't know if this is a bug for Info-read-node-name-1 (or -2) or a
> > bug for one of the `minibuffer.el' functions that it uses.  The code
> > is a bit hard to follow.
> >  
> > 
> > In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
> >  of 2008-10-03 on LENNART-69DE564
> > Windowing system distributor `Microsoft Corp.', version 5.1.2600
> > configured using `configure --with-gcc (3.4) --no-opt 
> > --cflags -Ic:/g/include
> > -fno-crossjumping'
> 
> FWIW, Emacs 22 also has the same problem, albeit differently:
> 
> (try-completion  "(el" 'Info-read-node-name-1) -> "(elisp"
> (all-completions "(el" 'Info-read-node-name-1) -> ("elisp")
> 
> So the problem is likely not with the `minibuffer.el' code.
> 
> In GNU Emacs 22.3.1 (i386-mingw-nt5.1.2600)
>  of 2008-09-06 on SOFT-MJASON
> Windowing system distributor `Microsoft Corp.', version 5.1.2600
> configured using `configure --with-gcc (3.4)'

I'm not sure this is a complete fix, but this code makes some progress at least.
It seems to DTRT, but I haven't tested it extensively. What I tried to do was
make it DTRT for `code' = t (in the first `cond' clause - I did not adjust the
second).

(defun Info-read-node-name-1 (string predicate code)
  (cond
    ;; First complete embedded file names.
    ((string-match "\\`(\\([^)]*\\)\\().*\\)?\\'" string) ; <=== 1
     (let* ((node (match-string 2 string))
            (ctwc 
             (completion-table-with-context
              "("
              (apply-partially
               'completion-table-with-terminator
               (or node ")")                              ; <=== 2
               (apply-partially 'Info-read-node-name-2
                                Info-directory-list
                                (mapcar 'car Info-suffix-list)))
              (match-string 1 string)                     ; <=== 3
              predicate
              code)))
       (cond ((eq code nil) ctwc)
             ((eq code t)
              (mapcar (lambda (file)                      ; <=== 4
                        (concat "(" file (or node ")")))
                      ctwc))
             (t t))))
    ;; If a file name was given, then any node is fair game.
    ((string-match "\\`(" string)
     (cond
       ((eq code nil) string)
       ((eq code t) nil)
       (t t)))
    ;; Otherwise use Info-read-node-completion-table.
    (t (complete-with-action
        code Info-read-node-completion-table string predicate))))

1. I changed the regexp so it matches also a possible closing paren and node
name.

2. I included the node name, if any, as the terminator in the call to
`completion-table-with-terminator'.

3. I changed (substring string 1) to (match-string 1 string), to pick up just
the file name, without closing paren.

4. I added the `mapcar' for a t `code', to wrap each of the file names in ():
("elisp") -> ("(elisp)"). Previously, the code just returned nil (meaning no
candidates) for `code' = t.

I did not try to look at or test the second clause of the first `cond':
(string-match "\\`(" string). I have no idea if it is correct - it's not even
clear to me what this clause is for. But it too returns nil (no candidates) for
a `code' value of t, so it seems suspect.

It would be great if someone else would please take a look and finish the job. I
think the code above is at least an improvement and doesn't introduce any
problems, but it should be checked, of course. 

HTH.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.