bug#4708: 23.1; completion-try-completion adds an extra $: $$HOMj

["Drew Adams" <drew.adams@oracle.com>]

> > emacs -Q
> > M-: (completion-try-completion "c:/some-dir/$HOMj" nil 16)
> > correctly returns: (c:/some-dir/$HOMEj" . 17)
> > M-: (completion-try-completion "c:/some-dir/$HOMj" nil 17)
> > returns: ("c:/some-dir/$$HOMj" . 18)
>  
> > That doesn't seem correct. Is it correct to have $$ here? If so, can
> > you please explain it a bit (why)?
> 
> File name entry assigns a special meaning to $ for envvars, 
> but in order to be able to refer to the file "$HOME", it offers
> an escape system where you write "$$HOME".  Since Emacs-21 or so,
> I've made this escape unnecessary in the case where the $ is
> used unambiguously (because there is no envvar of this name),
> so if you enter "$HOMj", Emacs know you don't refer to the file
> name contained in the envvar `HOMj', but to
> the file named $HOMj.

But there is no such file either. There is neither an envar that starts with
`$HOMj' nor a file whose name starts with `$HOMj'. No matter how you look at it,
there is no possible completion for `$HOMj'. Based on the doc string, I would
expect nil, to indicate that there is no possible completion.

> The completion code need to do such unescape/reescaping in 
> order to work properly and it currently re-escapes in a
> conservative way (i.e. it re-escapes $HOMj into $$HOMj even
> tho it's not strictly necessary).
> 
> File name completion is designed to result a string appropriate for
> substitute-in-file-name, and indeed
> 
>    (substitute-in-file-name "c:/some-dir/$$HOMj") ->
>    "c:/some-dir/$HOMj"
> 
> > Also, if this is correct behavior, then please explain this 
> > in the doc string of `completion-try-completion'.
> 
> It can't be described there, because that function applies to any kind
> of completion, whereas the above only applies to completions using
> read-file-name-internal.

You describe some of the foibles of the implementation, but you don't really
address the bug. It's not clear whether you intend to do that later or you are
trying to say that there is in fact no bug.

Code that calls `completion-try-completion' does not necessarily know what the
value of the TABLE arg is (i.e. whether it is equivalent to
`read-file-name-internal'), so it does not necessarily know whether this is
file-name completion or not.

`completion-try-completion' needs to have an unambiguous way to indicate that
there is no completion possible for the given input, even if it is the TABLE arg
that ultimately gives it that info. The doc says that nil is the way that
no-possible-completion is indicated.

But in this case nil is not returned. Instead, the return value can only be
interpreted, based on the doc string, as indicating that there is one
completion, "c:/some-dir/$$HOMj" (however that is to be intepreted: as a file
name that includes two `$' or as a file name that includes one `$' followed by
an env var). But there is in fact no legitimate completion.

There is no way that "c:/some-dir/$$HOMj" is a valid completion for
"c:/some-dir/$HOMj" - whether you regard the input as a file name that includes
a `$' or you regard it as an attempt to complete an env var. There simply is no
way that "c:/some-dir/$$HOMj" completes.

You can decide where the bug is: `read-file-name-internal',
`completion-try-completion', or somewhere else, but there is a bug if a value
claimed to represent a successful completion is returned when there is no valid
completion.

Or else there must be some way to tell from that returned value that it in fact
somehow represents "there is no completion" (the same as nil does). In that
case, let me know what that way is, and I'll test the return value using it. And
in that case the doc string also needs to document this how-to-tell info.

IOW, either the return value is correct and somehow indicates that there is no
completion - and in that case how to tell that? Or the return value is
incorrect, because it indicates a completion when there is none. Either way,
there is a bug.

I don't know how to handle this return value. My code currently checks for nil
as the (only) indicator of non-completion, and so it incorrectly interprets this
as successful completion.