bug#11718: 24.1.50; `all-completions' returns results with wrong case

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Michael Heerdegen]

Hello,

I use emacs-snapshot on Debian Linux ("GNU Emacs 24.1.50.1
(i486-pc-linux-gnu, GTK+ Version 3.4.2)\n of 2012-06-14 on zelenka,
modified by Debian").

I have a directory "~/Trash".  If I eval

   (let ((completion-ignore-case t))
     (all-completions "~/tra" 'read-file-name-internal 
                      'file-exists-p nil))

in emacs -Q, I get

  (#("trash/" 0 3 (face completions-common-part)))

Note the wrong lower case of the result.  In Emacs 23, however, I get

  ("Trash/")

that is, the right case.

Not sure if this is really a bug, but, at least, this change in
behavior is documented nowhere, and it causes a completion bug in
Icicles.


Thanks,

Michael.



In GNU Emacs 24.1.50.1 (i486-pc-linux-gnu, GTK+ Version 3.4.2)
 of 2012-06-14 on zelenka, modified by Debian
 (emacs-snapshot package, version 2:20120614-1)
Windowing system distributor `The X.Org Foundation', version 11.0.11201902
Configured using:
 `configure '--build' 'i486-linux-gnu' '--host' 'i486-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/24.1.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.1.50/site-lisp:/usr/share/emacs/site-lisp'
 '--without-compress-info' '--with-crt-dir=/usr/lib/i386-linux-gnu/'
 '--with-x=yes' '--with-x-toolkit=gtk3' '--with-imagemagick=yes'
 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu'
 'CFLAGS=-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000 -g -O2' 'LDFLAGS=-g
 -Wl,--as-needed -znocombreloc' 'CPPFLAGS=-D_FORTIFY_SOURCE=2''

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Stefan Monnier]

> I have a directory "~/Trash".  If I eval
>    (let ((completion-ignore-case t))
>      (all-completions "~/tra" 'read-file-name-internal 
>                       'file-exists-p nil))
> in emacs -Q, I get
>   (#("trash/" 0 3 (face completions-common-part)))
> Note the wrong lower case of the result.

While not strictly wrong, it is indeed an undesirable result.  I'll try
and see how to fix it.

> Not sure if this is really a bug, but, at least, this change in
> behavior is documented nowhere, and it causes a completion bug in
> Icicles.

Sounds like it hits a real Icicles bug: there are rather few guarantees
about the actual case of the returned string when completion-ignore-case
is set.  So while we do want to fix the problem, code should not
assume anything about the particular case of the return string (which
is only considered to be a "cosmetic" issue).


        Stefan

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Drew Adams]

> > I have a directory "~/Trash".  If I eval
> >    (let ((completion-ignore-case t))
> >      (all-completions "~/tra" 'read-file-name-internal 
> >                       'file-exists-p nil))
> > in emacs -Q, I get
> >   (#("trash/" 0 3 (face completions-common-part)))
> > Note the wrong lower case of the result.
> 
> While not strictly wrong,

See below.

> it is indeed an undesirable result. 
> I'll try and see how to fix it.
> 
> > Not sure if this is really a bug, but, at least, this change in
> > behavior is documented nowhere, and it causes a completion bug in
> > Icicles.
> 
> Sounds like it hits a real Icicles bug: there are rather few 
> guarantees about the actual case of the returned string when 
> completion-ignore-case is set.  So while we do want to fix the
> problem, code should not assume anything about the particular
> case of the return string (which is only considered to be a
> "cosmetic" issue).

I don't know what the Icicles connection is.

But is it not the case that `completing-read' should return an actual completion
candidate (or a string copy, but with the same case at least)?  If a candidate
is "Trash" and that candidate is chosen then it should not return "trash" or
"TRASH" or "trAsh", no?

Likewise, for `read-file-name': If the actual file-name candidate is "Trash"
then it should not return "trash", no?

I would think this would be true regardless of the value of
`completion-ignore-case'.  That variable should control only completion
_matching_, acting as a filter.  It should not affect/alter the completion
candidate that is returned.

To perform case-insensitive matching it would be permissible as a matching
implementation to uppercase or lowercase everything and then compare.  But such
an implementation should not provide an excuse to _return_ such a massaged
candidate.  The original candidate (or a string copy) should be returned.

`completion-ignore-case' should not affect the result in any way.  That's the
only proper interpretation of "no guarantees wrt the result": it should have no
effect on the result.  It should affect only whether one of the candidates
proposed matches your input.

Returning one of the actual completion candidates (or a `string=' copy), and not
returning its uncle or "halloween" or its mirror image or ... is not a
"cosmetic" issue.  So it seems to me, and I see nothing in either the doc or in
past Emacs behavior to contradict that.  Ignoring case during completion, just
like a PREDICATE arg, has only to do with matching.  Or so it should.

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Stefan Monnier]

> But is it not the case that `completing-read' should return an actual
> completion candidate (or a string copy, but with the same case at
> least)?

Usually, yes, but when quoting is involved, this is not so clear.
If the user typed C-x C-f $TMP/to TAB she liked "$TMP" so Emacs should
not replace it with "/var/private-tmp-f71dbe52628a3f83a77ab494817525c6/Total"
but with "$TMP/Total".  IOW some of the result should come from the
user's input and some of it from the completion table.

It's already difficult for Emacs to figure out that "tal" is what was
added, so currently it doesn't try to see that "/to" was changed into
"/To" and that this change is not a form of quoting and should hence be
reflected in the user's input.


        Stefan

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Michael Heerdegen]

"Drew Adams" <drew.adams@oracle.com> writes:

> I don't know what the Icicles connection is.

If you type C-x C-f ~ / t r a TAB, this is what we currently do:

  all-completions("~/tra" read-file-name-internal file-exists-p nil)
* icicle-all-completions("~/tra" read-file-name-internal file-exists-p nil)
  icicle-unsorted-file-name-prefix-candidates("~/tra")
  icicle-file-name-prefix-candidates("~/tra")
  icicle-prefix-complete-1()
  icicle-prefix-complete()

where `icicle-unsorted-file-name-prefix-candidates' looks like this:

--8<---------------cut here---------------start------------->8---
(defun icicle-unsorted-file-name-prefix-candidates (input)
  "Unsorted list of prefix completions for the current file-name INPUT.
When `icicle-expand-input-to-common-match' = 3 or 4, which implies
prefix auto-expansion, this also sets `icicle-common-match-string' to
the expanded common match of the input over all candidates."
  (condition-case nil
      (let* ((pred  (if (< emacs-major-version 23) default-directory
                      minibuffer-completion-predicate))
             (candidates
              (if (icicle-not-basic-prefix-completion-p)
                  (icicle-completion-all-completions
                   input minibuffer-completion-table pred
                   (length input)
                   (and (fboundp 'completion--field-metadata) ;Emacs24
                        (completion--field-metadata
                         (field-beginning))))
                (icicle-all-completions input minibuffer-completion-table pred
                                        icicle-ignore-space-prefix-flag)))
             (icicle-extra-candidates
              (icicle-remove-if-not ..4..))
             (icicle-proxy-candidates ..5..)
             (filtered-candidates ..16..))
        (when (and (memq icicle-expand-input-to-common-match '(3 4))
                   (consp filtered-candidates)) ..15..)
        (unless filtered-candidates  (setq icicle-common-match-string  nil))
        filtered-candidates)
    (quit (top-level))))
--8<---------------cut here---------------end--------------->8---


Thus, the minibuffer input is completed to ~/trash, although only
~/Trash exists.

Michael.

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Drew Adams]

> > But is it not the case that `completing-read' should return 
> > an actual completion candidate (or a string copy, but with
> > the same case at least)?
> 
> Usually, yes, but when quoting is involved, this is not so clear.
> If the user typed C-x C-f $TMP/to TAB she liked "$TMP" so Emacs should
> not replace it with 
> "/var/private-tmp-f71dbe52628a3f83a77ab494817525c6/Total"
> but with "$TMP/Total".

FWIW, the former is what Emacs did before you (someone) changed it, no?  E.g.
Emacs 22 (or 21 or 20 or ... 18), emacs -Q:

(let ((completion-ignore-case  t))
  (read-file-name "prompt: " nil "foobar"))

prompt: $HOME/dre TAB

changes the input to /drews-lisp-20/

Whereas Emacs 24 changes it to $HOME/drews-lisp-20/
with $HOME dimmed.

But I agree that the handling of env vars can seem to muddy the waters.  In any
case, the completion candidates themselves are relative file names, and their
case reflects the actual file names.  And that is so regardless of the platform
and regardless of `completion-ignore-case'.

IIUC, the candidates themselves do not include any of the $TMP stuff, whether
expanded or not.  In the case above there is only one matching candidate,
"drews-lisp-20" (which is a subdir of the root directory).  If that directory
were named "DrewsLisp" instead then it should presumably be expanded by Emacs 22
to /DrewsLisp/ and by Emacs 24 to $HOME/DrewsLisp/.

Even on a case-insensitive file system such as MS Windows, the resulting file
names should be, and have always been, the actual file names.  If the file or
dir is named TotoFoo then TotoFoo is what we should show and return to the user,
even when s?he types `tot TAB'.

The laxity wrt case is for the user, and only for matching.  It lets the user
type `tot' or `Tot' or `TOT' etc. to match `TotoFoo'.  It is not the completion
code and its return value that we want to be lax with, but the user.  It's about
user convenience.  The returned file name should still be correct, case
included.

> IOW some of the result should come from the
> user's input and some of it from the completion table.
> 
> It's already difficult for Emacs to figure out that "tal" is what was
> added, so currently it doesn't try to see that "/to" was changed into
> "/To" and that this change is not a form of quoting and 
> should hence be reflected in the user's input.

I cannot speak to the difficulty of a fix or how it is currently evaluated.

But it seems to me that Emacs _should_ not change the case of the candidates
themselves (whether file names or anything else).  The candidates supplied to
`completing-read' or computed by a function should be taken as is and returned
as chosen.  Perhaps with additional boundary text, but not with any case
changes.

To me, the mission of `completion-ignore-case' is limited to selection of
possible matches - it should do nothing except filter.  It should have no effect
on the returned choice.

IOW, I agree that `completion-ignore-case' should "guarantee nothing" about the
case of the result.  But the requirement is even stronger than that, IMO:
`c-i-c' has _nothing to do_ with the form of the result, including its case.

Whether the result is uppercase, lowercase, or mixed case should not be affected
by the value of `c-i-c'.  It should be decided by the completion function (e.g.
`read-file-name-internal') or the set of completions provided (e.g. obarray,
alist).

Do we disagree about this "should"?  I cannot speak to the difficulty of
implementation.  I am not arguing that it is easy to DTRT.  But it is not clear
whether you agree about what TRT is.  Do you think `c-i-g' should have any
bearing at all on the case of the result?  If so, then we disagree.

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Stefan Monnier]

Drew, I have no idea what you're hoping to get.  I already agreed before you
even sent a single message in this thread.
It's not like I'm rejecting a patch or something.


        Stefan


>>>>> "Drew" == Drew Adams <drew.adams@oracle.com> writes:

>> > But is it not the case that `completing-read' should return 
>> > an actual completion candidate (or a string copy, but with
>> > the same case at least)?
>> 
>> Usually, yes, but when quoting is involved, this is not so clear.
>> If the user typed C-x C-f $TMP/to TAB she liked "$TMP" so Emacs should
>> not replace it with 
>> "/var/private-tmp-f71dbe52628a3f83a77ab494817525c6/Total"
>> but with "$TMP/Total".

> FWIW, the former is what Emacs did before you (someone) changed it, no?  E.g.
> Emacs 22 (or 21 or 20 or ... 18), emacs -Q:

> (let ((completion-ignore-case  t))
>   (read-file-name "prompt: " nil "foobar"))

> prompt: $HOME/dre TAB

> changes the input to /drews-lisp-20/

> Whereas Emacs 24 changes it to $HOME/drews-lisp-20/
> with $HOME dimmed.

> But I agree that the handling of env vars can seem to muddy the waters.  In any
> case, the completion candidates themselves are relative file names, and their
> case reflects the actual file names.  And that is so regardless of the platform
> and regardless of `completion-ignore-case'.

> IIUC, the candidates themselves do not include any of the $TMP stuff, whether
> expanded or not.  In the case above there is only one matching candidate,
> "drews-lisp-20" (which is a subdir of the root directory).  If that directory
> were named "DrewsLisp" instead then it should presumably be expanded by Emacs 22
> to /DrewsLisp/ and by Emacs 24 to $HOME/DrewsLisp/.

> Even on a case-insensitive file system such as MS Windows, the resulting file
> names should be, and have always been, the actual file names.  If the file or
> dir is named TotoFoo then TotoFoo is what we should show and return to the user,
> even when s?he types `tot TAB'.

> The laxity wrt case is for the user, and only for matching.  It lets the user
> type `tot' or `Tot' or `TOT' etc. to match `TotoFoo'.  It is not the completion
> code and its return value that we want to be lax with, but the user.  It's about
> user convenience.  The returned file name should still be correct, case
> included.

>> IOW some of the result should come from the
>> user's input and some of it from the completion table.
>> 
>> It's already difficult for Emacs to figure out that "tal" is what was
>> added, so currently it doesn't try to see that "/to" was changed into
>> "/To" and that this change is not a form of quoting and 
>> should hence be reflected in the user's input.

> I cannot speak to the difficulty of a fix or how it is currently evaluated.

> But it seems to me that Emacs _should_ not change the case of the candidates
> themselves (whether file names or anything else).  The candidates supplied to
> `completing-read' or computed by a function should be taken as is and returned
> as chosen.  Perhaps with additional boundary text, but not with any case
> changes.

> To me, the mission of `completion-ignore-case' is limited to selection of
> possible matches - it should do nothing except filter.  It should have no effect
> on the returned choice.

> IOW, I agree that `completion-ignore-case' should "guarantee nothing" about the
> case of the result.  But the requirement is even stronger than that, IMO:
> `c-i-c' has _nothing to do_ with the form of the result, including its case.

> Whether the result is uppercase, lowercase, or mixed case should not be affected
> by the value of `c-i-c'.  It should be decided by the completion function (e.g.
> `read-file-name-internal') or the set of completions provided (e.g. obarray,
> alist).

> Do we disagree about this "should"?  I cannot speak to the difficulty of
> implementation.  I am not arguing that it is easy to DTRT.  But it is not clear
> whether you agree about what TRT is.  Do you think `c-i-g' should have any
> bearing at all on the case of the result?  If so, then we disagree.

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Drew Adams]

> Drew, I have no idea what you're hoping to get.  I already 
> agreed before you even sent a single message in this thread.
> It's not like I'm rejecting a patch or something.

Sorry, it was not clear to me that you agreed.  Glad you do.

FWIW, this is what sounded equivocal (unclear) to me:

> While not strictly wrong, it is indeed an undesirable result.
        ^^^^^^^^^^^^^^^^^^
> I'll try and see how to fix it.

> So while we do want to fix the problem, code should not
> not assume anything about the particular case of the return
  ^^^^^^^^^^^^^^^^^^^
> string (which is only considered to be a "cosmetic" issue).
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I did hear you say clearly that you want to fix the problem.

But I also (apparently mis-) heard you saying that code should not expect the
case of the return string to correspond to the completions provided by the
COLLECTION arg.  It sounded like your position might be that all bets are off
wrt the result.

Sorry for any noise.  Glad you agree.

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Stefan Monnier]

> I did hear you say clearly that you want to fix the problem.

Yes.

> But I also (apparently mis-) heard you saying that code should not expect the
> case of the return string to correspond to the completions provided by the
> COLLECTION arg.  It sounded like your position might be that all bets are off
> wrt the result.

The problem here is that you misunderstand what is the COLLECTION
argument in the case at hand.  It is *not* a table that completes
filenames as they exist in the file-system.  If you want that, then use
completion-file-name-table.  `read-file-name-internal' is a completion
table that does other things to handle substitute-in-file-name rewrite
(i.e. typically env-var expansion, $$-quoting, // and ~/ truncation);
part of that makes it that you should not put too much faith in the case
of the returned string.


        Stefan

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Drew Adams]

> The problem here is that you misunderstand what is the COLLECTION
> argument in the case at hand.  It is *not* a table that completes
> filenames as they exist in the file-system.  If you want 
> that, then use completion-file-name-table.

The case at hand is the one Michael presented.  All that is done in that code is
to pass `minibuffer-completion-table' and `minibuffer-completion-predicate' to
`all-completions'.  The former happens to be `read-file-name-internal' in this
case, but the code does not impose that (it comes from `read-file-name', which
the code uses).

> `read-file-name-internal' is a completion table that does
> other things to handle substitute-in-file-name rewrite (i.e.
> typically env-var expansion, $$-quoting, // and ~/ truncation);

A priori I do not have a problem with any of that.  Au contraire, I think.

> part of that makes it that you should not put too much faith
> in the case of the returned string.

That's the part that I'm not sure follows.  Logically.

But I repeat that I cannot speak to the implementation, and I do not claim that
it is easy to get it right.  I am only saying that file-name completion - even
with env-var expansion etc. - _should_ not change the case of file names for the
return value.  It is a bug if it it does, though not a major bug.

(Again, "should" does not imply that it is easy to fix.)

I'm not sure we disagree.  And I hope you can fix this eventually, even if it is
not a high priority.  And I understand from your posts that this might not be
easy, file names being mixed up with env-var substitution et tout le reste.

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Stefan Monnier]

> it is easy to get it right.  I am only saying that file-name
> completion - even with env-var expansion etc. - _should_ not change
> the case of file names for the return value.  It is a bug if it it
> does, though not a major bug.

And what I'm saying is that a completion table which handles quoting
such as read-file-name-internal returns file names which are partly
taken from the completion input and partly from the file-system.
So, what you perceive as "changing the case" is no change at all, it's
just that the string you expect to come from the file-system actually
comes from the user's input.

> (Again, "should" does not imply that it is easy to fix.)

And again, "should" only in the sense that it's better, not that it's
needed for correctness.


        Stefan

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Michael Heerdegen]

Hello Stefan,

> And what I'm saying is that a completion table which handles quoting
> such as read-file-name-internal returns file names which are partly
> taken from the completion input and partly from the file-system.

I'm still learning how that completion table stuff is working.

I think, since Icicles doesn't respect quoting all the time anyway, we
should suffice with this completion table:

(completion-table-in-turn #'completion--embedded-envvar-table
                          #'completion-file-name-table)  ; not: completion--file-name-table

which would circumvent the problem.

But I have another question:

(all-completions "/usr/share/emacs/24.1.50/lisp/$HO"
                 #'completion--embedded-envvar-table)

returns ("HOME").  What I would expect is ("$HOME").  Is this a bug?  If
not, is there a way to get ("$HOME")?


Thank you for your patience,

Michael.

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[Stefan Monnier]

> (all-completions "/usr/share/emacs/24.1.50/lisp/$HO"
>                  #'completion--embedded-envvar-table)
> returns ("HOME").  What I would expect is ("$HOME").  Is this a bug?

No, it's a choice in completion--embedded-envvar-table.

> If not, is there a way to get ("$HOME")?

Sure.  Take completion--embedded-envvar-table and change it to add the
$ in front of its output and to subtract 1 from the `boundaries'
it returns.


        Stefan

bug#11718: 24.1.50; `all-completions' returns results with wrong case

[npostavs]

found 11718 24.3
tags 11718 fixed
close 11718 24.4
quit

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hello,
>
> I use emacs-snapshot on Debian Linux ("GNU Emacs 24.1.50.1
> (i486-pc-linux-gnu, GTK+ Version 3.4.2)\n of 2012-06-14 on zelenka,
> modified by Debian").
>
> I have a directory "~/Trash".  If I eval
>
>    (let ((completion-ignore-case t))
>      (all-completions "~/tra" 'read-file-name-internal 
>                       'file-exists-p nil))
>
> in emacs -Q, I get
>
>   (#("trash/" 0 3 (face completions-common-part)))
>
> Note the wrong lower case of the result.  In Emacs 23, however, I get
>
>   ("Trash/")

Emacs 24.3 gives (#("trash/" 0 3 (face completions-common-part)))
Emacs 24.4 gives (#("Trash/" 0 3 (face completions-common-part)))