bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Daniel Mendler]

`(all-completions "~/.config/~man" #'read-file-name-internal)` returns
the list `("/.config/~man")`. The file completion table incorrectly
recognizes "/.config/~man" as the username instead of only "~man".

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Eli Zaretskii]

> From: Daniel Mendler <mail@daniel-mendler.de>
> Date: Fri, 30 Jul 2021 00:17:40 +0200
> 
> `(all-completions "~/.config/~man" #'read-file-name-internal)` returns
> the list `("/.config/~man")`. The file completion table incorrectly
> recognizes "/.config/~man" as the username instead of only "~man".

Why do you think that is incorrect?  AFAIU, this is a well-documented
behavior: "~" anywhere in the name discards everything before it.
Exactly like "/" does.

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Lars Ingebrigtsen]

Daniel Mendler <mail@daniel-mendler.de> writes:

> `(all-completions "~/.config/~man" #'read-file-name-internal)` returns
> the list `("/.config/~man")`. The file completion table incorrectly
> recognizes "/.config/~man" as the username instead of only "~man".

I'm unable to reproduce this problem, and you haven't used `M-x
report-emacs-bug', so I don't know what system you're seeing this on.
I'm getting the expected on Debian/bullseye:

(all-completions "~/.config/~man" #'read-file-name-internal)
=> (#("/.config/~man/" 0 13 (face completions-common-part)))

That is, it's expanding ~man to ~man/ since it interprets that bit as a
user name.

And, of course:

(all-completions "~/.config/~man/" #'read-file-name-internal)
=> ("tr/" "zh/" "CACHEDIR.TAG" "de/" "ru/" "ro/" "cat3/" "it/" "pt/" "./" "cat7/" "zh_TW/" "sr/" "fr.ISO8859-1/" "hr/" "cat6/" "fi/" "ja/" "index.db" "cat1/" "cat2/" "cat5/" "cat8/" "vi/" "ko/" "nl/" "uk/" "sl/" "../" "zh_CN/" "id/" "hu/" "da/" "sv/" "es/" "fr.UTF-8/" "pt_BR/" "cs/" "pl/" "sk/" "cat4/" "fr/")

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Daniel Mendler]

On 7/30/21 1:47 PM, Lars Ingebrigtsen wrote:
>> `(all-completions "~/.config/~man" #'read-file-name-internal)` returns
>> the list `("/.config/~man")`. The file completion table incorrectly
>> recognizes "/.config/~man" as the username instead of only "~man".
> 
> I'm unable to reproduce this problem, and you haven't used `M-x
> report-emacs-bug', so I don't know what system you're seeing this on.
> I'm getting the expected on Debian/bullseye:
> 
> (all-completions "~/.config/~man" #'read-file-name-internal)
> => (#("/.config/~man/" 0 13 (face completions-common-part)))

No, this is exactly the problem I meant (You reproduced it).
"/.config/~man" is an invalid completion string. It is not a valid path
nor a valid username on my system. The file completion table should not
return invalid completion candidates.

> And, of course:
> 
> (all-completions "~/.config/~man/" #'read-file-name-internal)
> => ("tr/" "zh/" "CACHEDIR.TAG" "de/" "ru/" "ro/" "cat3/" "it/" "pt/" "./" "cat7/" "zh_TW/" "sr/" "fr.ISO8859-1/" "hr/" "cat6/" "fi/" "ja/" "index.db" "cat1/" "cat2/" "cat5/" "cat8/" "vi/" "ko/" "nl/" "uk/" "sl/" "../" "zh_CN/" "id/" "hu/" "da/" "sv/" "es/" "fr.UTF-8/" "pt_BR/" "cs/" "pl/" "sk/" "cat4/" "fr/")

Yes, of course. This works correctly and as expected.

Daniel

bug#49777: bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Daniel Mendler]

A few more examples for comparison:

(all-completions "~/etc/~man" 'read-file-name-internal); /etc/~man/, bug
(all-completions "~/etc/~foo" 'read-file-name-internal); nil, ok
(all-completions "/etc/~man" 'read-file-name-internal); man/, ok
(all-completions "/etc/~man/" 'read-file-name-internal); ~man files, ok
(all-completions "~/etc/~man/" 'read-file-name-internal); ~man files, ok

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Lars Ingebrigtsen]

Daniel Mendler <mail@daniel-mendler.de> writes:

> On 7/30/21 1:47 PM, Lars Ingebrigtsen wrote:
>>> `(all-completions "~/.config/~man" #'read-file-name-internal)` returns
>>> the list `("/.config/~man")`. The file completion table incorrectly
>>> recognizes "/.config/~man" as the username instead of only "~man".
>> 
>> I'm unable to reproduce this problem, and you haven't used `M-x
>> report-emacs-bug', so I don't know what system you're seeing this on.
>> I'm getting the expected on Debian/bullseye:
>> 
>> (all-completions "~/.config/~man" #'read-file-name-internal)
>> => (#("/.config/~man/" 0 13 (face completions-common-part)))
>
> No, this is exactly the problem I meant (You reproduced it).

But you said it returned "/.config/~man" for you (while it returns
"/.config/~man/" for me) (note trailing slash)...

> "/.config/~man" is an invalid completion string. It is not a valid path
> nor a valid username on my system.

Oh, you don't have a user named "man" on your system?  That explains why
we're getting different results.  And I'm unable to reproduce that here:

(all-completions "~/.config/~notexist" #'read-file-name-internal)
=> nil

So what kind of system are you on?

> The file completion table should not return invalid completion
> candidates.

The confusing bit for me is why there's a "/" at the start of the
returned value, to be honest.  ".config/~man/" should be the correct
value here, I think...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> (all-completions "~/.config/~notexist" #'read-file-name-internal)
> => nil

And note that this is the same as

(read-file-name-internal "~/.config/~notexist" nil t)
=> nil

(read-file-name-internal "~/.config/~man" nil t)
=> (#("/.config/~man/" 0 13 (face completions-common-part)))

and note that this is an internal function not to be used by users.

So I don't see anything misbehaving here...  What's the real problem
you're seeing (when not using internal Emacs functions)?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Daniel Mendler]

On 7/30/21 2:50 PM, Lars Ingebrigtsen wrote:
> But you said it returned "/.config/~man" for you (while it returns
> "/.config/~man/" for me) (note trailing slash)...

Yes, this was a typo from my side, sorry for that. Please look at the
other examples I've sent.

So to describe this better: (all-completions "~/etc/~man"
'read-file-name-internal) returns ("/etc/~man/"). In contrast
(all-completions "/usr/~man" 'read-file-name-internal) returns ("man/").
"/etc/~man/ is neither a valid user name nor a valid file path. I have
the user "man" on my system of course (I am also on a recent Debian).

So the problem here is that in to even call the completion table with
"~/etc/~man", the completion boundary is placed such that the string
behind the first "~" is completed. If I read Eli correctly, he says that
this is the expected behavior, while I would have expected that the
completion boundary is placed behind the second "~", such that the
completion table does not return odd looking results like "/etc/~man".
Furthermore the string before the first "~" can be considered shadowed.
So maybe the problem here is not with the results returned by
`all-completions` but rather with the completion boundaries. I think
this needs further investigation from my side.

Daniel

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Daniel Mendler]

On 7/30/21 3:12 PM, Lars Ingebrigtsen wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
>> (all-completions "~/.config/~notexist" #'read-file-name-internal)
>> => nil
> (read-file-name-internal "~/.config/~notexist" nil t)
> => nil

Yes, this is what I expect. I see the same.

> (read-file-name-internal "~/.config/~man" nil t)
> => (#("/.config/~man/" 0 13 (face completions-common-part)))

I see this too and it may be correct behavior. But then I wonder why the
completion boundaries are placed as they are.

> and note that this is an internal function not to be used by users.

I am not looking at this as a user. I am investigating odd completion
results in my Vertico completion UI (on GNU ELPA), when completing file
names. If you type "~/etc/~man", you see these odd candidates
"/etc/~man", where "/etc/" is actually part of the shadowed path. So to
say it differently, I don't expect the shadowed path to take part in the
completion.

Maybe Stefan can chime in here and explain some of the background. I've
corresponded with him before a few times regarding how the completion
machinery works internally.

Daniel

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Lars Ingebrigtsen]

Daniel Mendler <mail@daniel-mendler.de> writes:

> I am not looking at this as a user. I am investigating odd completion
> results in my Vertico completion UI (on GNU ELPA), when completing file
> names. If you type "~/etc/~man", you see these odd candidates
> "/etc/~man", where "/etc/" is actually part of the shadowed path. So to
> say it differently, I don't expect the shadowed path to take part in the
> completion.

Ah, right.

> Maybe Stefan can chime in here and explain some of the background. I've
> corresponded with him before a few times regarding how the completion
> machinery works internally.

Right.  Stefan, in short -- the thing we're wondering is whether this is
correct: 

(read-file-name-internal "~/foo/~man" nil t)
=> (#("/foo/~man/" 0 9 (face completions-common-part)))

(read-file-name-internal "./foo/~man" nil t)
=> (#("man/" 0 3 (face completions-common-part)))

I'd kinda expect both to give the same result (i.e., the latter result)?

(And "man" is a valid user in this scenario.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2021-07-30 19:18:53] wrote:
> Daniel Mendler <mail@daniel-mendler.de> writes:
>> I am not looking at this as a user. I am investigating odd completion
>> results in my Vertico completion UI (on GNU ELPA), when completing file
>> names. If you type "~/etc/~man", you see these odd candidates
>> "/etc/~man", where "/etc/" is actually part of the shadowed path. So to
>> say it differently, I don't expect the shadowed path to take part in the
>> completion.
>
> Ah, right.
>
>> Maybe Stefan can chime in here and explain some of the background. I've
>> corresponded with him before a few times regarding how the completion
>> machinery works internally.
>
> Right.  Stefan, in short -- the thing we're wondering is whether this is
> correct: 
>
> (read-file-name-internal "~/foo/~man" nil t)
> => (#("/foo/~man/" 0 9 (face completions-common-part)))

Yup,. that's wrong.

> (read-file-name-internal "./foo/~man" nil t)
> => (#("man/" 0 3 (face completions-common-part)))

This one looks right.

> I'd kinda expect both to give the same result (i.e., the latter result)?

Agreed.


        Stefan

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> (read-file-name-internal "~/foo/~man" nil t)
>> => (#("/foo/~man/" 0 9 (face completions-common-part)))
>
> Yup,. that's wrong.

I'm completely unfamiliar with this code...

(defun completion-table-with-quoting (table unquote requote)
...
           (boundary (car (completion-boundaries ustring table pred "")))
           (completions
            (completion--twq-all
             string ustring completions boundary unquote requote))

ustring here is "~man", and the boundary is 1, so completion--twq-all
returns "/foo/~man" (because it stripped off 1 character at the start).

But it should have stripped off "~/foo/~", I think?  Somehow?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Daniel Mendler]

On 7/31/21 2:07 PM, Lars Ingebrigtsen wrote:
> ustring here is "~man", and the boundary is 1, so completion--twq-all
> returns "/foo/~man" (because it stripped off 1 character at the start).
> 
> But it should have stripped off "~/foo/~", I think?  Somehow?

Yes. I think the problem here is the wrong completion boundary. The
completion boundary should be placed such that the shadowed path is not
part of the completion. In this case the completion boundary should be
behind the second "~".

Daniel

bug#49776: 28.0.50; Incorrect username completion results reported by `read-file-name-interal`

[Lars Ingebrigtsen]

Daniel Mendler <mail@daniel-mendler.de> writes:

>> ustring here is "~man", and the boundary is 1, so completion--twq-all
>> returns "/foo/~man" (because it stripped off 1 character at the start).
>> 
>> But it should have stripped off "~/foo/~", I think?  Somehow?
>
> Yes. I think the problem here is the wrong completion boundary. The
> completion boundary should be placed such that the shadowed path is not
> part of the completion. In this case the completion boundary should be
> behind the second "~".

Yup.  But my understanding of the completion code is extremely limited,
so hopefully somebody who understands it better can propose a fix...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no