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

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

[Drew Adams]

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'
 

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

[Drew Adams]

> 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)'

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

[Drew Adams]

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

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

[Stefan Monnier]

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

This fact has simply never been true in general.
It's true as long as the completion table is not a function, but
otherwise all bets are off.

In practice the main case where this "invariant" does not hold is when
the completion table is the one for file names, where all-completions
returns names that do not "full" (i.e. do not include the directory
name).  But other cases exist.

For code that needs to live with this problem (e.g. minibuffer.el),
Emacs-23 introduces a new way to call the completion table function with
a `boundary' argument.


        Stefan

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

[Drew Adams]

> From: Stefan Monnier Sent: Sunday, October 05, 2008 4:36 PM
> > 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).
> 
> This fact has simply never been true in general.
> It's true as long as the completion table is not a function, but
> otherwise all bets are off.

It has been true in general. It has just been false for a few exceptions. I
think it should be true always. It should be possible to bring those exceptions
into the fold. And if in some case there must truly be an exception, so be it.
But that is not so for all of the cases today where the invariants are not
respected. `Info-read-node-name-1' is a case in point - see below for code.

There is something to be gained by eliminating such exceptions, beyond just
consistency. As I said,

 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 should be possible at least for all deterministic TABLE functions that have
a finite range, that is, for most functional TABLE args used in practice.

> In practice the main case where this "invariant" does not hold is when
> the completion table is the one for file names, where all-completions
> returns names that do not "full" (i.e. do not include the directory
> name).

Why shouldn't it be true in that case also? It isn't now, but it could be, could
it not? File name completion could be made to keep the same user behavior it has
now and still respect the invariants I mentioned.

> But other cases exist.

I don't think it's right to allow it. It's especially not right not to
discourage it. It just makes it impossible for Lisp code to reason about or act
generally regarding completions. I don't see why it's necessary to allow it at
all.

> For code that needs to live with this problem (e.g. minibuffer.el),
> Emacs-23 introduces a new way to call the completion table 
> function with a `boundary' argument.

Why should `Info-read-node-name-1' in particular "need to live with this
problem"? Why shouldn't it return, for a CODE of t, a valid list of completions
satisfying the invariants I mentioned? Why not do it right?

Ideally (this part is for the wish list), we would provide completion not only
for input such as `(ema', giving all-completions `("(emacs)" "(emacs-mime)")',
but also for input such as `(emacs)Mac', giving all-completions `("(emacs)Mac
Directories" "(emacs)Mac Environment"...)'.

This is analogous to file-name completion, with directories as analogs of Info
file names such as "(emacs)". Today there is no completion even for `(emacs)Mac
OS', though you can hit RET with that input to go to node `Mac OS'. That would
be like saying that you can complete "/ema" to "/emacs" and you can hit RET to
visit "/emacs/COPYING", but you cannot complete "/emacs/COP".

Until that wish-list item is implemented we should at least make the Info
file-name completion DTRT for `all-completions' (i.e. fix this bug).

FWIW, here is the code I'm using now to do that for Emacs 23:

(defun Info-read-node-name-1 (string predicate code)
  (cond ((string-match "\\`(\\([^)]*\\))\\'" string) ; e.g. (emacs)
         (cond ((eq code nil) string)
               ((eq code t) (list string))
               (t t)))
        ((string-match "\\`(\\([^)]*\\)\\'" string) ; e.g. (emacs
         (let ((ctwc (completion-table-with-context
                      "("
                      (apply-partially
                       'completion-table-with-terminator ")"
                       (apply-partially 'Info-read-node-name-2
                                        Info-directory-list
                                        (mapcar 'car Info-suffix-list)))
                      (match-string 1 string)
                      predicate
                      code)))
           (cond ((eq code nil) ctwc)
                 ((eq code t)
                  (mapcar (lambda (file) (concat "(" file ")")) ctwc))
                 (t t))))
        ((string-match "\\`(" string) ; e.g. (emacs)Mac OS - just punt.
         (cond ((eq code nil) string)
               ((eq code t) nil)
               (t t)))
        ;; Otherwise use Info-read-node-completion-table - e.g. Mac OS
        (t (complete-with-action code Info-read-node-completion-table
                                 string predicate))))

And FWIW, here is the code I'm using now for Emacs 22:

(defun Info-read-node-name-1 (string predicate code)
  (cond ((string-match "\\`(\\([^)]*\\))\\'" string) ; e.g. (emacs) or
(emacs-mime)
         (cond ((eq code nil) string)
               ((eq code t) (list string))
               (t t)))
        ((string-match "\\`(\\([^)]*\\)\\'" string) ; e.g. (emacs
         (let ((file (match-string 1 string)))
           (cond ((eq code nil)
                  (let ((comp (try-completion
                                file 'Info-read-node-name-2
                                (cons Info-directory-list
                                      (mapcar #'car Info-suffix-list)))))
                    (cond ((eq comp t) (concat string ")"))
                          (comp (concat "(" comp)))))
                 ((eq code t)
                  (mapcar (lambda (file) (concat "(" file ")"))
                          (all-completions file 'Info-read-node-name-2
                                           (cons Info-directory-list
                                                 (mapcar #'car
Info-suffix-list)))))
                 (t nil))))
        ((string-match "\\`(" string) ; e.g. (emacs)Mac OS or (jlkj - just punt.
         (cond ((eq code nil) string)
               ((eq code t) nil)
               (t t)))
        ;; Otherwise use Info-read-node-completion-table - e.g. Mac OS
        ((eq code nil)
         (try-completion string Info-read-node-completion-table predicate))
        ((eq code t)
         (all-completions string Info-read-node-completion-table predicate))
        (t (test-completion string Info-read-node-completion-table predicate))))

I think this is an improvement and it or something like it should be
incorporated as a fix for this bug. Again, please test etc.

I hope you'll reconsider the policy wrt support for the invariants I suggested,
instead of encouraging (yes) n'importe quoi. If there is truly no implementation
choice in some particular case, then so be it, but it shouldn't be the policy to
not even try to do something clean. FWIW, I'm not convinced there are any cases
where there is really no choice.

We should also add a guideline to the doc encouraging people to respect the
invariants mentioned when they write code with functional TABLE args.

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

[Stefan Monnier]

> It has been true in general. It has just been false for a few exceptions.

Different uses of "in general".

> I think it should be true always.

There are 2 different needs.  For the user's convenience, the
*Completions* buffer should only list "foo for fot" rather than
"/toto/foo /toto/for /toto/fot" and similarly for various other
completion cases.  Currently all-completions returns the first list and
the `boundary' thingy can indicate that the missing prefix is "/toto/".
You want to change that so that all-completions returns the second list,
which still requires some way to figure out that the part that should be
stripped from display is "/toto/".
Efficiency and history are on the side of the current behavior.

> It should be possible to bring those exceptions into the fold.

For what benefit?

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

Use the `boundary' thingy.  And note that this is new in Emacs-23.
In previous versions of Emacs there simply was no standardized way to do
that reliably.

>> In practice the main case where this "invariant" does not hold is when
>> the completion table is the one for file names, where all-completions
>> returns names that do not "full" (i.e. do not include the directory
>> name).

> Why shouldn't it be true in that case also? It isn't now, but it could
> be, could it not? File name completion could be made to keep the same
> user behavior it has now and still respect the invariants I mentioned.

Because the output of all-completions has historically been used for the
*Completions* output, so aesthetic and convenience may impose
other constraints.

>> For code that needs to live with this problem (e.g. minibuffer.el),
>> Emacs-23 introduces a new way to call the completion table 
>> function with a `boundary' argument.

> Why should `Info-read-node-name-1' in particular "need to live with
> this problem"?

Info-read-node-name-1 does not "live with this problem".  On the
contrary it makes good use of this freedom.

> Why shouldn't it return, for a CODE of t, a valid list
> of completions satisfying the invariants I mentioned? Why not do
> it right?

I don't know what you mean by "a CODE of t".

> Ideally (this part is for the wish list), we would provide completion
> not only for input such as `(ema', giving all-completions `("(emacs)"
> "(emacs-mime)")', but also for input such as `(emacs)Mac', giving
> all-completions `("(emacs)Mac Directories" "(emacs)Mac
> Environment"...)'.

It's indeed on the wishlist.  I'm pretty sure there's a TODO about it in
the code.  But it has noting to do with the dicsussion at hand.

>                   (mapcar (lambda (file) (concat "(" file ")")) ctwc))

That's not correct when completing "(/usr/shar".

> I hope you'll reconsider the policy wrt support for the invariants
> I suggested, instead of encouraging (yes) n'importe quoi.

I have banged my head against this problem for a long time, while
working on the minibuffer.el code (and its predecessors), so I don't
expect to change my mind any time soon.  The only thing you'd accomplish
with your proposal is "change", but it would just shift the problems
from one place to another without any actual benefit.


        Stefan

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

[Drew Adams]

> From: Stefan Monnier Sent: Monday, October 06, 2008 7:37 AM
> > It has been true in general. It has just been false for a 
> > few exceptions.
> 
> Different uses of "in general".

Yes. It is true almost all of the time, but not all of the time.

I think it is false for some cases where it could be made true.

> > I think it should be true always.
> 
> There are 2 different needs.  For the user's convenience, the
> *Completions* buffer should only list "foo for fot" rather than
> "/toto/foo /toto/for /toto/fot" and similarly for various other
> completion cases.

Agreed.

> Currently all-completions returns the first list and
> the `boundary' thingy can indicate that the missing prefix is 
> "/toto/".

In passing: The `boundary' thingy needs to be documented. It is not obvious how
to use it or how it works.

> You want to change that so that all-completions returns the 
> second list,

No, not necessarily. I only want `try-completion', `all-completions' and
`test-completion' to be on the same page, in the sense of the invariants I
mentioned.

I believe it should be possible for the functions that perform completion to
DTRT even for cases like file-name completion, so that the user's input
corresponds to the candidates in *Completions* and to `all-completions' and
`try-completion'.

I'm not sure how that should best be done, but I think it should be possible to
take care of the directory part without it becoming part of each completion
candidate. That's kind of what we do now anyway. Perhaps you have an idea about
how to do it?

> which still requires some way to figure out that the part that should be
> stripped from display is "/toto/". Efficiency and history are on the side
> of the current behavior.
> 
> > It should be possible to bring those exceptions into the fold.
> 
> For what benefit?

Consistency. Ability to manipulate or reason about completion code and treat it
in a general manner. See next. 

> >  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).
> 
> Use the `boundary' thingy.  And note that this is new in Emacs-23.
> In previous versions of Emacs there simply was no 
> standardized way to do that reliably.

Can you explain a bit how to do that?
Will that enable respect of the invariants I mentioned?

> >> In practice the main case where this "invariant" does not 
> >> hold is when the completion table is the one for file names,
> >> where all-completions returns names that do not "full" (i.e.
> >> do not include the directory name).
> 
> > Why shouldn't it be true in that case also? It isn't now, 
> > but it could be, could it not? File name completion could be made
> > to keep the same user behavior it has now and still respect the
> > invariants I mentioned.
> 
> Because the output of all-completions has historically been 
> used for the *Completions* output,

That's OK. What I'm thinking of is having both `all-completions' and
*Completions* display, say, relative file names, as now, and having
`try-completion' also use and return a relative file name, but have them all
somehow deal with the directory. The directory can be passed separately, no?

> so aesthetic and convenience may impose other constraints.

I'm not claiming a simple, immediate solution. I'm asking you to think about it.

> >> For code that needs to live with this problem (e.g. minibuffer.el),
> >> Emacs-23 introduces a new way to call the completion table 
> >> function with a `boundary' argument.
> 
> > Why should `Info-read-node-name-1' in particular "need to live with
> > this problem"?
> 
> Info-read-node-name-1 does not "live with this problem".  On the
> contrary it makes good use of this freedom.

I don't see it as such good use. To me it is a bug. I sent a fix.

> > Why shouldn't it return, for a CODE of t, a valid list
> > of completions satisfying the invariants I mentioned? Why not do
> > it right?
> 
> I don't know what you mean by "a CODE of t".

Argument `code' = t in the source code (and in the fix I sent).

> > Ideally (this part is for the wish list), we would provide 
> > completion not only for input such as `(ema', giving
> > all-completions `("(emacs)" "(emacs-mime)")', but also for
> > input such as `(emacs)Mac', giving all-completions
> > `("(emacs)Mac Directories" "(emacs)Mac Environment"...)'.
> 
> It's indeed on the wishlist.  I'm pretty sure there's a TODO 
> about it in the code.  But it has noting to do with the
> dicsussion at hand.

No, admittedly it is not directly related.

The question at hand is how to handle completion of just the Info file-name
part, e.g. `(emacs-mime)'. To me, the user's input should match what s?he sees
in *Completions* (with parens), and they should both reflect both
`all-completions' and `try-completion'.

> >                   (mapcar (lambda (file) (concat "(" file ")")) ctwc))
> 
> That's not correct when completing "(/usr/shar".

Is that possible? Could you elaborate? What should the code do here?

I don't claim that the fix I sent is 100%. It seems to DTRT, but it might
require some tweaking.

In any case, the `all-completions' part of the current code is currently broken
and requires some fix. It doesn't make sense for input of `(emacs)' with CODE =
t to return nil (no completions). `all-completions' should be able to return the
complete set of file-name completions in that case. IOW, (Info-read-node-name-1
"(emacs)" nil t) should not return nil, meaning no completions - it should
return the list of all completions, ("(emacs)"), just as (Info-read-node-name-1
"(emacs)" nil nil) returns "(emacs)".

> > I hope you'll reconsider the policy wrt support for the invariants
> > I suggested, instead of encouraging (yes) n'importe quoi.
> 
> I have banged my head against this problem for a long time, while
> working on the minibuffer.el code (and its predecessors), so I don't
> expect to change my mind any time soon.  The only thing you'd 
> accomplish with your proposal is "change", but it would just shift
> the problems from one place to another without any actual benefit.

Could you maybe explain some of your thoughts on it? Share some of your internal
head-banging behind the design and implementation? I don't doubt that you've
thought it over - how about sharing some of the thoughts and considerations?

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

[Stefan Monnier]

> No, not necessarily. I only want `try-completion', `all-completions' and
> `test-completion' to be on the same page, in the sense of the invariants I
> mentioned.

Then I missed those invariants.
Could you spell them out again one by one?


        Stefan


PS: Regarding the behavior of info completion for "(emacs)", it can't be
done right until someone writes the relevant code.  Basically the
current code doesn't know how to do completion in this case, so
try-completion can't return anything use, and neither can
all-completions, or test-completion.  The current code instead just
tries to return something that's safe: try-completion returns the string
unchanged, which indicates that it's a valid completion and that there's
more completions out there; test-completion returns t to basically say
the same; and all-completions returns nil so that
minibuffer-completing-help doesn't give some bogus list of completions.

The try-completion and test-completion outputs are fairly reasonable.
The all-completions one is less satisfactory since returning nil seems
to imply that the input is not a valid completion, whereas
try-completion and test-completion say that it is.  Returning a list
containing a copy of the input wouldn't be right either since it would
imply that the current input is the sole completion.  Basically, we'd
need a special `dont-know' return value, but it doesn't seem worth
the trouble.

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

[Drew Adams]

> From: Stefan Monnier Sent: Monday, October 06, 2008 7:06 PM
>
> > > You want to change that so that all-completions returns
> > > the second list [i.e. list of absolute, not relative
> > > file names]
> 
> > No, not necessarily. I only want `try-completion', 
> > `all-completions' and `test-completion' to be on the same
> > page, in the sense of the invariants I mentioned.
> 
> Then I missed those invariants.
> Could you spell them out again one by one?

Actually, it was you who first used the term "invariants", referring presumably
to the behavior I had described/requested. ;-)

But without at first using that term, I did speak of invariants:

1. User's input should be a prefix of what it is completed to in the minibuffer,
which should be the same as the common prefix of all its completions, the result
of `try-completion':

> 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)".]

2. `all-completions' elements correspond to `test-completion':

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

3. `all-completions' elements correspond to `try-completion':

> 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 [see #1]). 

I don't know how independent these are; there is probably some overlap. But they
seem like good rules to shoot for.

> PS: Regarding the behavior of info completion for "(emacs)", 

I assume by that you mean completion of things like "(emacs)Mac O" to
"(emacs)Mac OS" and even completion of "(emacs)" to all of the Emacs manual
nodes (a la completion of a directory /some/dir/ for files).

> it can't be done right until someone writes the relevant code.
> Basically the current code doesn't know how to do completion
> in this case, so try-completion can't return anything use[ful?],
> and neither can all-completions, or test-completion.

Yes, I realize that. 

The code I sent lets you complete just the file-name part - "(ema" to "(emacs"
and "(emacs-" to "(emacs-mime"). That's as far as it goes.

I even thought about temporarily moving to the manual and doing a node
completion there, then prefixing each node by "(emacs)" etc. But aside from the
ugliness of that approach and my laziness, I wasn't sure of the behavior I
wanted.

I think now that a straight analogy to file-name completion wouldn't be bad:
"(emacs)Fo" in the minibuffer and all of the Emacs manual node names in
*Completions* - without "(emacs)" prefix - just like for file names. Just as
with file names, you could delete the default directory (manual's file name -
"(emacs)" from the minibuffer if you wanted. Just as with file names, you could
change the directory (manual's file) explicitly in the minibuffer.

Yes, I know that these musings might seem to contradict some of the invariants
mentioned above. I think that if Info node completion behaved just like
file-name completion I'd be happy. Dunno.

> The current code instead just tries to return something
> that's safe:

Safe = ?  I don't follow.

> try-completion returns the string unchanged, which indicates
> that it's a valid completion and that there's
> more completions out there; test-completion returns t to basically say
> the same;

I think they are OK.

> and all-completions returns nil so that
> minibuffer-completing-help doesn't give some bogus list
> of completions.

That's not good, I think. nil means there are no completions.

> The try-completion and test-completion outputs are fairly reasonable.

Yes.

> The all-completions one is less satisfactory since returning nil seems
> to imply that the input is not a valid completion, whereas
> try-completion and test-completion say that it is.

We agree.

> Returning a list containing a copy of the input wouldn't
> be right either since it would imply that the current
> input is the sole completion.

Agreed.

That's why I fixed it to return the complete set of manuals (files): ("(emacs)"
"(emacs-mime").

But it would be good to be able to complete beyond "(emacs)" to all of the Emacs
manual nodes - like completing "/some/d" to "/some/dir/" and then completing
that to all of the directory's files.

> Basically, we'd need a special `dont-know' return value,
> but it doesn't seem worth the trouble.

I don't follow you there. What do you mean?

All of the manuals (files) are known, and all of the nodes in each manual are
known. So such completion should be possible.

 - Drew

P.S. Could you explain a little about boundaries and what you meant about using
that feature. This is not clear to me.

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

[Stefan Monnier]

> 1. User's input should be a prefix of what it is completed to in the
> minibuffer, which should be the same as the common prefix of all its
> completions, the result of `try-completion':

Let's keep "completion in the minibuffer" for another thread and let's
focus on try0completion, all-completions, and test-competion.

>> 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)".]

I believe this is indeed a valid invariant.  At least I can't remember
any case where this needed to be broken.

> 2. `all-completions' elements correspond to `test-completion':

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

This is not a valid invariant.  E.g. let's say you want a completion
table for "png files".  You'll probably want the completion to happen
"one directory at a time", like all other file completions in Emacs, so
all-completions will have to be able to return directories, even though
these are not png files and will hence be rejected by test-completion.

> 3. `all-completions' elements correspond to `try-completion':

>> 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 [see #1]). 

As discussed, this would imply that the code that builds *Completions*
figure out which part of the prefix to drop.  So this is the feature
about which I said:

   I have banged my head against this problem for a long time, while
   working on the minibuffer.el code (and its predecessors), so I don't
   expect to change my mind any time soon.  The only thing you'd accomplish
   with your proposal is "change", but it would just shift the problems
   from one place to another without any actual benefit.

So of the 3 invariants you request, the first already holds, the second
cannot always hold, and the third will not always hold because it would
introduce problems elsewhere.

> P.S. Could you explain a little about boundaries and what you meant
> about using that feature. This is not clear to me.

minibuffer.el says:

;; - The `action' can be (additionally to nil, t, and lambda) of the form
;;   (boundaries . SUFFIX) in which case it should return
;;   (boundaries START . END).  See `completion-boundaries'.
;;   Any other return value should be ignored (so we ignore values returned
;;   from completion tables that don't know about this new `action' form).

So, (funcall TABLE STRING PRED '(boundary . "")) should return
(boundaries START END) where END should be (length STRING) or
nil, and START will be the length of the prefix of STRING which is
missing from the all-completions output (e.g. for file completion,
START should be more or less equivalent to
(length (file-name-directory STRING))).
Note that some completion tables may not know about this new
`boundaries' argument, and will hence return something different from
(boundaries START END), so any code using this new feature should treat
any other return value as equivalent to (boundaries 0 (length STRING)).

As for your invariant 3, you should be able to recover it if you use
something like:

  (defun drew-all-completions (string table pred)
    (if (not (functionp table))
        (all-completions string table pred)
      (let* ((bs (funcall table string pred '(boundaries . "")))
             (prefix (and (eq 'boundaries (car-safe bs))
                          (integerp (car-safe (cdr bs)))
                          (substring string 0 (cadr bs)))))
        (mapcar (lambda (s) (concat prefix s))
                (all-completions string table pred)))))

I'm pretty sure this code won't work as-is, but hopefully, you get
the idea.


        Stefan

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

[Drew Adams]

Sorry, this is a bit long. Please be patient.

> > 1. User's input should be a prefix of what it is completed to in the
> > minibuffer, which should be the same as the common prefix of all its
> > completions, the result of `try-completion':
> 
> Let's keep "completion in the minibuffer" for another thread and let's
> focus on try0completion, all-completions, and test-competion.

OK, but I'm not sure it won't need to be brought in here at some point. And I
get the impression that you do kind of bring it in, below.

> > 2. `all-completions' elements correspond to `test-completion':
> 
> >> 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.
> 
> This is not a valid invariant.  E.g. let's say you want a completion
> table for "png files".  You'll probably want the completion to happen
> "one directory at a time", like all other file completions in Emacs,

I guess you're saying that file-name completion is within a given directory. If
not, it sounds like you're bringing in "completion in the minibuffer".

If so, then I don't know what you mean by this:

> so all-completions will have to be able to return directories, 

Do you mean return the name of a subdir?

> even though these are not png files and will hence be rejected by
> test-completion.

I see your point, I think. In that case - that is, if you are saying that a
"png-file" cannot be a directory, we are no longer talking about "like all other
file completions in Emacs" - where subdirectory names *are* treated as file
names.

IOW, if you don't want to include directories in the type "png-files", then
subdirectories would not be among the completions - it's a choice.

But you could define "png-file" (for completion purposes) to either (1) include
directories or (2) allow a directory prefix in the file name.

I think you've extrapolated from the file-completion case, but you've ignored
the fact that Emacs file-completion does treat directory names as valid file
names (for completion). Is "png-file" like file-completion or something
different? Do you want to let directories be completed or not? These are design
decisions for png-file completion. But I see no reason that "png-file" couldn't
be defined to be able to complete as one would like.

In normal file completion also, (all-completions...) can return subdir names as
some of the completions. And (test-completion...) returns t for any of those
subdir names.

Perhaps I'm missing some of what you're saying here; dunno.

> > 3. `all-completions' elements correspond to `try-completion':
> 
> >> 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 [see #1]). 
> 
> As discussed, this would imply that the code that builds *Completions*
> figure out which part of the prefix to drop.

The code that builds *Completions* is part of the discussion of "completion in
the minibuffer" - that is, what the user sees, as opposed to what happens for
`all-completions', `try-completion', and `test-completion'. I thought you wanted
to postpone discussion of that.

Anyway, I don't see why *Completions* would drop part of the prefix, unless by
"prefix" you are referring to part of what is in the minibuffer beyond the
prompt - e.g. the directory part for file-name completion. To me, that is *not*
a prefix of any of the completions - it is additional information needed to do
the completing. The fact that a directory name appears to the left of a file
name is almost accidental; it does not make the directory name into a prefix of
the file name.

Really, two pieces of information are being passed to the completion function:
(1) the file name to complete and (2) the directory in which to find the file
whose name you want to complete. Note that #2 is not tied to being a directory
name (string) - it is the directory itself that is needed to look up the
constituent files and their names. The (implementation/design) problem is that
completion functions are predisposed to act on only a string (e.g. file name)
and predicate - there is no explicit provision for passing additional
information that might be needed.

The Info manual/node vs file-name directory/file analogy is a good one for
reasoning about this, I think. In file-name completion, you can erase the
directory part from the minibuffer. The directory info is treated (e.g. passed)
separately; it is not part of the "prefix" for `all-completions' or
`try-completion'.

A user can type a different directory in the minibuffer, and that is taken into
account, but that happens in `find-file' or `read-file-name', not in
`all-completions' and `try-completion'. 

The same approach could be taken for Info and elsewhere as is used for file-name
completion - don't treat all of what is in the minibuffer beyond the prompt as
"prefix" in the completion sense. Use it (or any other context information) as
additional information that lets the completion function do its job. How to pass
that info to the completion function is a design/implementation problem, but it
shouldn't impact the invariants we're discussing.

> So this is the feature about which I said:
> 
>    I have banged my head against this problem for a long time, while
>    working on the minibuffer.el code (and its predecessors), 
>    so I don't expect to change my mind any time soon.  The only thing 
>    you'd accomplish with your proposal is "change", but it would just
>    shift the problems from one place to another without any actual
>    benefit.

I don't see the connection with what is above. That's a far jump.

> So of the 3 invariants you request, the first already holds, 
> the second cannot always hold, and the third will not always hold 
> because it would introduce problems elsewhere.

See above.

> > P.S. Could you explain a little about boundaries and what you meant
> > about using that feature. This is not clear to me.
> 
> minibuffer.el says:
> 
> ;; - The `action' can be (additionally to nil, t, and lambda) 
> of the form
> ;;   (boundaries . SUFFIX) in which case it should return
> ;;   (boundaries START . END).  See `completion-boundaries'.
> ;;   Any other return value should be ignored (so we ignore 
> values returned
> ;;   from completion tables that don't know about this new 
> `action' form).

By "it" in "it should return" I think you mean the function that is the value of
COLLECTION, but that could be made clearer. Similarly, the description of
COLLECTION in the doc string of `all-completions' and `try-completion' (which is
referred to from the doc for `completing-read' should say something more about
the return value of a functional COLLECTION - something beyond just "Whatever it
returns becomes the value of `all-completions'.

Another problem is that COLLECTION, in the `all-completions' doc, is described
as receiving arguments STRING, PREDICATE, and t. Nothing is said about
COLLECTION being able to handle an ACTION arg value of (boundaries . SUFFIX).
Someone writing a COLLECTION function that uses the `boundaries' feature needs
to know how this works. I really think some more work needs to be put into the
doc for this (by you ;-)).

> So, (funcall TABLE STRING PRED '(boundary . "")) should return
> (boundaries START END) where END should be (length STRING) or
> nil, and START will be the length of the prefix of STRING which is
> missing from the all-completions output (e.g. for file completion,
> START should be more or less equivalent to
> (length (file-name-directory STRING))).

Thanks; that's what I thought and saw by debugging, but this helps.

Why treat the directory as a prefix? You are in effect passing it around
separately, specially, but you are assuming unnecessarily that it will be a
prefix. Really, it's just additional info that is needed for the completion
process.

"(emacs)" in Info would be treated the same way, and there too you could
consider it a prefix, so the same mechanism could be used to do the wish-list
completion we discussed, no?

But I don't see why this extra information should always be considered a prefix.
It could be anything really. It's just something that lets COLLECTION (aka
TABLE) do its job and something that we do *not* want to appear as a prefix of
each completion. In the case of file-name and Info node completion you *could*
consider it as in some sense an invisible prefix (but not a prefix that is part
of each completion), but you need not think of it that way. 

How about making the boundaries feature less dependent on treating this separate
info as a (pseudo-)prefix? Instead of a string, it could be any type. A
directory-name string is used to represent a directory, and knowledge of that
directory is used to come up with the proper completions, but the fact that the
directory's name can appear as a prefix to the file name that is the real
completion is little more than an accident. What's really happening is that the
completion function (i.e. COLLECTION) is using the directory object to complete
its STRING input. 

The problem, as I see it, is that among STRING, PREDICATE, and ACTION, there is
no explicit provision for passing the additional info (e.g. directory). So we
have fudged. We use PREDICATE for the dir. Or we fudge ACTION so it can sneak in
the dir as a pseudo-prefix. Fudging ACTION is maybe better than fudging
PREDICATE, and it leaves more room for manoeuver and engenders less conflict,
but it is still a fudge. And if ACTION is fudged to do this, that still doesn't
mean that the extra info should be treated as a prefix (string, start, end,
etc.).

Shouldn't we fix this right, so the extra info that the COLLECTION function
needs is passed as an arg in its own right? I agree with your changes (still in
progress, IIUC) to clean up PRED so it is always a predicate. But it seems to me
that sneaking the extra info in in the form of a "prefix" string inside ACTION
is  also an ugly hack. Why don't we try to come up with something that fits
legacy but also DTRT?

Whether or not today we can always somehow consider or treat the extra info
needed as in some sense a "prefix" of each completion, if we do it right then
tomorrow, at least, there will be cases where info is passed and used that is
really, really in no sense a "prefix". Don't you agree?

I suspect that, at least historically, the treatment of this as a "prefix" might
be bound up with the treatment of base-size and highlighting of the common
prefix. Is that right? If so, I'd suggest cutting that cord now. Prefix
highlighting is something else again, and involves a true prefix that is part of
each completion. And other kinds of completion (e.g. substring) need different
kinds of such highlighting. I don't know if `boundaries' really has anything to
do with base-size and prefix highlighting, but I somehow got that feeling.

> Note that some completion tables may not know about this new
> `boundaries' argument, and will hence return something different from
> (boundaries START END), so any code using this new feature 
> should treat any other return value as equivalent to (boundaries 0
> (length STRING)).

Got it.

> As for your invariant 3, you should be able to recover it if you use
> something like:
> 
>   (defun drew-all-completions (string table pred)
>     (if (not (functionp table))
>         (all-completions string table pred)
>       (let* ((bs (funcall table string pred '(boundaries . "")))
>              (prefix (and (eq 'boundaries (car-safe bs))
>                           (integerp (car-safe (cdr bs)))
>                           (substring string 0 (cadr bs)))))
>         (mapcar (lambda (s) (concat prefix s))
>                 (all-completions string table pred)))))
> 
> I'm pretty sure this code won't work as-is, but hopefully, you get
> the idea.

Yes, thanks; I think so.

Don't get me wrong. I think you've done a great job moving the completion stuff
to Lisp. I think there's still room for improvement, but I admit that there are
some parts of it that I don't understand well, so I might be missing things. 

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

[Stefan Monnier]

> "png-file" cannot be a directory, we are no longer talking about "like
> all other file completions in Emacs" - where subdirectory names *are*
> treated as file names.

See http://en.wikipedia.org/wiki/PNG to learn what is a PNG file.

> I think you've extrapolated from the file-completion case, but you've ignored
> the fact that Emacs file-completion does treat directory names as valid file
> names (for completion).

You talk as if there was only a single form of file-name completion in
Emacs, even though it's clearly not the case: it depends on the kind of
file name you want to get.  It may be restricted to directories, to
files of a particular kind, etc...

> Is "png-file" like file-completion or something different? Do you want
> to let directories be completed or not? These are design decisions for
> png-file completion. But I see no reason that "png-file" couldn't be
> defined to be able to complete as one would like.

If you want to be able to enter /usr/share/foo/bar/toto/foo.png with
completion, that means that all-completions should either return all png
files in your file system (this is impractical) or that it returns both
png files and directories, so that you can complete "/u" to "/usr/",
etc...  But that doesn't mean that test-completion should accept "/usr/".

> Perhaps I'm missing some of what you're saying here; dunno.

What I'm saying is that your invariant is too restrictive and it makes
sense to break it sometimes.  So it's not a valid invariant and code
should not rely on it being true.

> "(emacs)" in Info would be treated the same way, and there too you
> could consider it a prefix, so the same mechanism could be used to do
> the wish-list completion we discussed, no?

Very much so.

> But I don't see why this extra information should always be considered
> a prefix.  It could be anything really. It's just something that lets
> COLLECTION (aka TABLE) do its job and something that we do *not* want
> to appear as a prefix of each completion. In the case of file-name and
> Info node completion you *could* consider it as in some sense an
> invisible prefix (but not a prefix that is part of each completion),
> but you need not think of it that way.

I have no clue what you're talking about.

> How about making the boundaries feature less dependent on treating
> this separate info as a (pseudo-)prefix? Instead of a string, it could

Because it wouldn't provide the info you need to write drew-all-completions.

> The problem, as I see it, is that among STRING, PREDICATE, and ACTION,
> there is no explicit provision for passing the additional info
> (e.g. directory). So we have fudged. We use PREDICATE for the dir.

No: "we useD PREDICATE for the dir".  This is being phased out.

> Or we fudge ACTION so it can sneak in the dir as
> a pseudo-prefix. Fudging ACTION is maybe better than fudging
> PREDICATE, and it leaves more room for manoeuver and engenders less
> conflict, but it is still a fudge. And if ACTION is fudged to do this,
> that still doesn't mean that the extra info should be treated as
> a prefix (string, start, end, etc.).

Neither should be fudged.  The extra info can be passed via buffer-local
variables, or stored inside the function (e.g. using lexical-let).

> Shouldn't we fix this right, so the extra info that the COLLECTION
> function needs is passed as an arg in its own right? I agree with your
> changes (still in progress, IIUC) to clean up PRED so it is always
> a predicate. But it seems to me that sneaking the extra info in in the
> form of a "prefix" string inside ACTION is  also an ugly hack. Why
> don't we try to come up with something that fits legacy but also DTRT?

Oh, is that what you're talking about?  No you completely misunderstand,
there's no relationship between the "prefix" stuff and the directory
info that used to be fudged into PREDICATE.

That directory info that used to be passed in PREDICATE is now passed
via buffer-local variables instead.


        Stefan

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

[Drew Adams]

> No you completely misunderstand, there's no relationship
> between the "prefix" stuff and the directory info that
> used to be fudged into PREDICATE.
> 
> That directory info that used to be passed in PREDICATE is
> now passed via buffer-local variables instead.

Right. I did misunderstand that. You use the boundaries thing to pass along the
context "(" for Info file name "(em", but you don't use it to pass the directory
for file-name completion - and I do see (and did, but forgot) how you pass the
directory in the latter case. My bad about that part.

I was following my analogy and thought that you did the same thing for both,
using the directory part as a boundary/contextual "prefix" of the relative file
name.

But IIUC, there is no invalidation of the invariants for file-name completion.
The directory used for completion is separate and kept separate from the input,
the completion candidates, and the completion result, which are all relative
names. The result of hitting RET for `read-file-name' is an absolute file name,
but the result of the completion itself - e.g. the result of
`read-file-name-internal', is just the relative name.

If you agree about that, then let's come back to "(em", which is a case, we
agree, where the invariants are currently invalidated. IIUC, you say it is a
case where the invariants *must* be invalidated. My question is why. Why
couldn't we treat this completion the same way we treat file-name completion?

I'm not saying that we must fix this for Info for Emacs 23.1 - I agreed that
such generalized Info completion is for the wishlist now. I'm only asking why it
shouldn't be the policy that the invariants are respected. Is there any case
where they logically *cannot* be respected?

I think that's what I'm missing: an example where there is no way to avoid a
mismatch between a completion per `try-completion' and a completion per
`all-completions'.

It seems to me that the invalidation for "(em" was introduced by treating "(" as
a prefix (using the boundaries mechanism) and not treating "(emacs)" the same
way we treat directories for file-name completion. That might be all we want to
do for now, but is Info completion a case where there is no choice but to
invalidate the invariants?

IIUC, completion of "(em" in Info now treats the "(" part as contextual boundary
info to be temporarily ignored while completing against a list of Info file
names. For that treatment, you use `boundaries'. 

And it is that treatment that introduces a difference between the completions of
`all-completions', which are Info file names such as "emacs-mime", and the
completions of `try-completions', which are names such as "(emacs-mime)". (In
Emacs 22, before `boundaries', a different treatment was used here, which also
invalidated the invariants.) Is this description of the situation correct?

Please think of what I'm writing not as disputing but as trying to understand. I
believe you that you know what you're talking about here. ;-) I just want to
understand it better.

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

[Stefan Monnier]

> Right. I did misunderstand that. You use the boundaries thing to pass
> along the context "(" for Info file name "(em", but you don't use it
> to pass the directory for file-name completion - and I do see (and
> did, but forgot) how you pass the directory in the latter case. My bad
> about that part.

> I was following my analogy and thought that you did the same thing for
> both, using the directory part as a boundary/contextual "prefix" of
> the relative file name.

> But IIUC, there is no invalidation of the invariants for file-name
> completion.

Your 3rd invariant is invalidated because all-completions does not
return the directory part of a completion.

> If you agree about that, then let's come back to "(em", which is a case, we
> agree, where the invariants are currently invalidated. IIUC, you say it is a
> case where the invariants *must* be invalidated. My question is why. Why
> couldn't we treat this completion the same way we treat file-name completion?

We do treat it identically.


        Stefan

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

[Drew Adams]

> > I was following my analogy and thought that you did the 
> > same thing for both, using the directory part as a
> > boundary/contextual "prefix" of the relative file name.
> 
> > But IIUC, there is no invalidation of the invariants for
> > file-name completion.
> 
> Your 3rd invariant is invalidated because all-completions does not
> return the directory part of a completion.

Yes and no. If you use find-file, then yes, you're right, because it calls, in
effect, try-completion passing an absolute file name:

(try-completion "c:/mydir/icicles." 'read-file-name-internal nil) gives
"c:/mydir/icicles.el", whereas, as you say, all-completions uses relative file
names.

But if you call try-completion directly using a relative file name, then it,
just like all-completions, returns the completed relative file name - completed
in the default directory.

(try-completion "icicles." 'read-file-name-internal nil) gives "icicles.el".

That was my point. If the directory information is passed separately, not
included in the file name, then try-completion and all-completions use the same
set of completions (relative file names). There is no invalidation of the
invariant - the two coexist harmoniously - the completions for try-completion
and for all-completions are the same.

> > If you agree about that, then let's come back to "(em", 
> > which is a case, we agree, where the invariants are
> > currently invalidated.  IIUC, you say it is a
> > case where the invariants *must* be invalidated. My 
> > question is why.

I still have the same question - why?

> > Why couldn't we treat this completion the same way we treat 
> > file-name completion?
> 
> We do treat it identically.

Not if I understand correctly. Isn't it true that we use the boundary thing
(with prefix "(") for the Info file/node completion, and we don't use it for
file-name completion? And doesn't the use of that feature invalidate the
invariant?

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

[Stefan Monnier]

>> > I was following my analogy and thought that you did the 
>> > same thing for both, using the directory part as a
>> > boundary/contextual "prefix" of the relative file name.
>> > But IIUC, there is no invalidation of the invariants for
>> > file-name completion.
>> Your 3rd invariant is invalidated because all-completions does not
>> return the directory part of a completion.

> But if you call try-completion directly using a relative file name,
> then it, just like all-completions, returns the completed relative
> file name - completed in the default directory.

Not if the relative file name includes a slash.

> (try-completion "icicles." 'read-file-name-internal nil) gives "icicles.el".

That's just a happy corner case.  We're discussing the general case.

>> > Why couldn't we treat this completion the same way we treat 
>> > file-name completion?
>> We do treat it identically.
> Not if I understand correctly. Isn't it true that we use the boundary thing
> (with prefix "(") for the Info file/node completion, and we don't use it for
> file-name completion?

We use it for file-name completion just the same.
If you're in the particular case where the file name has no directory
component, then the prefix is the empty string so you may get fooled
into thinking that it's not used, but it is.


        Stefan

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

[Drew Adams]

> >> > I was following my analogy and thought that you did the 
> >> > same thing for both, using the directory part as a
> >> > boundary/contextual "prefix" of the relative file name.
> >> > But IIUC, there is no invalidation of the invariants for
> >> > file-name completion.
> >> Your 3rd invariant is invalidated because all-completions does not
> >> return the directory part of a completion.
> 
> > But if you call try-completion directly using a relative file name,
> > then it, just like all-completions, returns the completed relative
> > file name - completed in the default directory.
> 
> Not if the relative file name includes a slash.
> 
> > (try-completion "icicles." 'read-file-name-internal nil) 
> > gives "icicles.el".
> 
> That's just a happy corner case.  We're discussing the general case.

Why couldn't that be the general case.

You seem to be reasoning based on the current implementation (e.g. that's not
the way it is), not based on the general discussion here.

> >> > Why couldn't we treat this completion the same way we treat 
> >> > file-name completion?
> >> We do treat it identically.
> > Not if I understand correctly. Isn't it true that we use the boundary
> > thing (with prefix "(") for the Info file/node completion, and we 
> > don't use it for file-name completion?
> 
> We use it for file-name completion just the same.

But do we need to? That's the question. Is it necessary to use boundaries in a
situation like this?

> If you're in the particular case where the file name has no directory
> component, then the prefix is the empty string so you may get fooled
> into thinking that it's not used, but it is.

You seem to be just repeating how it's done now, and not listening to my
argument. Is there any reason that `boundaries' needs to be used for file-name
completion? Couldn't `try-completion' always use relative file names?

That's the question - is there any reason why try-completion and all-completions
*must* be incompatible (in the sense of the invariant being invalidated) for
file-name completion. You seem to be ignoring that question and just responding
that in the current implementation they are incompatible.

You said earlier that the directory is passed separately, which it is. What
information is needed besides the relative file name and the directory in which
completion is to be done? If they are all that are needed for both
try-completion and all-completions, then why do they need to be incompatible?

I'm asking about *required* behavior/design - what is necessary and what
alternatives are possible, not just how it happens to be implemented now.

I'll give up after this. *Seems* like you're not really trying, but I'm not
jumping to that conclusion yet. If I'm misunderstanding something, I'd like to
learn it.

bug#1085: marked as done (23.0.60; all-completions, try-completion inconsistent: Info-read-node-name-1)

[Emacs bug Tracking System]

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


Your message dated Sat, 15 Aug 2009 18:19:04 -0400
with message-id <87vdkolnzb.fsf@cyd.mit.edu>
and subject line RE: bug#1085: 23.0.60; all-completions, try-completion inconsistent: Info-read-node-name-1
has caused the Emacs bug report #1085,
regarding 23.0.60; all-completions, try-completion inconsistent: Info-read-node-name-1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com
immediately.)


-- 
1085: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=1085
Emacs Bug Tracking System
Contact owner@emacsbugs.donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 4436 bytes --]

From: "Drew Adams" <drew.adams@oracle.com>
To: <emacs-pretest-bug@gnu.org>
Subject: 23.0.60; all-completions, try-completion inconsistent: Info-read-node-name-1
Date: Sat, 4 Oct 2008 15:58:20 -0700
Message-ID: <00b801c92674$b294d550$0200a8c0@us.oracle.com>

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'
 




[-- Attachment #3: Type: message/rfc822, Size: 1208 bytes --]

From: Chong Yidong <cyd@stupidchicken.com>
To: 1085-done@emacsbugs.donarmstrong.com
Subject: RE: bug#1085: 23.0.60; all-completions, try-completion inconsistent: Info-read-node-name-1
Date: Sat, 15 Aug 2009 18:19:04 -0400
Message-ID: <87vdkolnzb.fsf@cyd.mit.edu>

Since discussion of this has come to a stop long ago, and there are no
planned changes to the new completion design, I'm closing this bug.