[PATCH] Fix hostname completion on MS Windows

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix hostname completion on MS Windows

Jim Porter
This patch resolves an MS Windows-specific issue with file name completion for Tramp: currently, when trying to complete a Tramp hostname (or method/user), it fails to match anything on MS Windows (however, completing the remote path works ok). This is because `file-name-completion' provides handlers with an absolute path to match against, but on MS Windows, that means the path starts with the volume letter, e.g. "c:/ssh:host:/path/to/file".

The attached patch fixes things for me; it's somewhat hacky, since it tries to recognize the mutilated path string as shown above. However, changing that would (probably) be an invasive change to how core Emacs handles file name completion. This also mirrors the behavior of `tramp-drop-volume-letter', so hopefully it's ok. I *think* this only applies to windows-nt systems and not cygwin (which usually uses *nix-style paths), but I don't have Cygwin installed so I didn't try it there.

I don't have copyright assignment papers filled out, but hopefully this patch is small enough that that's not necessary. If you think it is, just let me know and I'll get that handled.

As a small addendum: this patch still doesn't fix things when `default-directory' is a UNC path (e.g. "\\server\mnt\"), but the same issue applies to `tramp-drop-volume-letter', and this case should be rare in practice anyway.

- Jim

0001-tramp.el-tramp-completion-file-name-regexp-default.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix hostname completion on MS Windows

Michael Albinus
Jim Porter <[hidden email]> writes:

Hi Jim,

> This patch resolves an MS Windows-specific issue with file name
> completion for Tramp: currently, when trying to complete a Tramp
> hostname (or method/user), it fails to match anything on MS Windows
> (however, completing the remote path works ok). This is because
> `file-name-completion' provides handlers with an absolute path to
> match against, but on MS Windows, that means the path starts with the
> volume letter, e.g. "c:/ssh:host:/path/to/file".

I don't mind to apply such a patch, but I haven't heard about this kind
of file name completion to "c:/ssh:host:/path/to/file". Could you pls
show an example, and what you have typed in order to get this?

Best might be to enable Tramp traces after loading Tramp, like

--8<---------------cut here---------------start------------->8---
(require 'trace)
(dolist (elt (all-completions "tramp-" obarray 'functionp))
  (trace-function-background (intern elt)))
(untrace-function 'tramp-read-passwd)
--8<---------------cut here---------------end--------------->8---

Then run the test, and disable traces afterwards

--8<---------------cut here---------------start------------->8---
(untrace-all)
--8<---------------cut here---------------end--------------->8---

There will be a new buffer *trace-output*, which shall tell us the
story.

> The attached patch fixes things for me; it's somewhat hacky, since it
> tries to recognize the mutilated path string as shown above.

That would be OK, but perhaps we make the volume letter optional? Like

--8<---------------cut here---------------start------------->8---
(when (eq system-type 'windows-nt)
    "\\(?:[[:alpha:]]:\\)?")
--8<---------------cut here---------------end--------------->8---

> However, changing that would (probably) be an invasive change to how
> core Emacs handles file name completion. This also mirrors the
> behavior of `tramp-drop-volume-letter', so hopefully it's ok. I
> *think* this only applies to windows-nt systems and not cygwin (which
> usually uses *nix-style paths), but I don't have Cygwin installed so I
> didn't try it there.

I know, that Eli Zaretskii (maintainer of Emacs) has committed several
patches in order to drop such a volume letter for remote file names, but
I don't remember where they have been applied. Perhaps, this was an
Emacs 28 change.

> I don't have copyright assignment papers filled out, but hopefully
> this patch is small enough that that's not necessary. If you think it
> is, just let me know and I'll get that handled.

This patch is small enough (the upper limit are ~15 lines), but in
general it is a good idea to sign copyright papers for future
changes. To start the assignment process you should download
https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
and return the completed information to the address at the top.

> As a small addendum: this patch still doesn't fix things when
> `default-directory' is a UNC path (e.g. "\\server\mnt\"), but the same
> issue applies to `tramp-drop-volume-letter', and this case should be
> rare in practice anyway.

Yep, we could handle this case afterwards. If needed.

> - Jim

Best regards, Michael.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix hostname completion on MS Windows

Jim Porter
On Thu, Apr 8, 2021 at 5:40 AM Michael Albinus <[hidden email]> wrote:
> I don't mind to apply such a patch, but I haven't heard about this kind
> of file name completion to "c:/ssh:host:/path/to/file". Could you pls
> show an example, and what you have typed in order to get this?

Starting with "emacs -Q":
M-: (require 'tramp)
C-x C-f /ss TAB

I'd expect completion to append an "h" to the path I'm typing, and if
I hit TAB again, to show "ssh:" and "sshx:" as possible completions.
The same issue happens if I type:

C-x C-f /sshx:serv TAB

I'd expect this to append "er:" so I have "/sshx:server:".[1]

> Best might be to enable Tramp traces after loading Tramp, like... [snip]

I've attached the trace output, though it might not be very useful,
since it didn't show where I think things are going wrong. To the best
of my knowledge, here's what's happening:

1) I type C-x C-f /ss TAB
2) Eventually, this calls (file-name-completion "ss" "/"
file-exists-p). All good so far.
3) The first thing `file-name-completion' does with the directory[2]
("/") is to call `expand-file-name' on it. This is a problem because
on MS Windows, `expand-file-name' will prepend the volume letter, so
now our directory is "c:/". Emacs is now effectively trying to
complete the file name "c:/ss", which Tramp's completion regexp
doesn't recognize.

> > The attached patch fixes things for me; it's somewhat hacky, since it
> > tries to recognize the mutilated path string as shown above.
>
> That would be OK, but perhaps we make the volume letter optional? Like
>
> --8<---------------cut here---------------start------------->8---
> (when (eq system-type 'windows-nt)
>     "\\(?:[[:alpha:]]:\\)?")
> --8<---------------cut here---------------end--------------->8---

That seems reasonable. I've attached an updated patch.

> I know, that Eli Zaretskii (maintainer of Emacs) has committed several
> patches in order to drop such a volume letter for remote file names, but
> I don't remember where they have been applied. Perhaps, this was an
> Emacs 28 change.

It looks like `file-name-completion' still adds the volume letter by
calling `expand-file-name' on Emacs 28. I didn't see any changes to
`file-name-completion' since Emacs 27.

> This patch is small enough (the upper limit are ~15 lines), but in
> general it is a good idea to sign copyright papers for future
> changes. To start the assignment process you should download
> https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
> and return the completed information to the address at the top.

Done.

> > As a small addendum: this patch still doesn't fix things when
> > `default-directory' is a UNC path (e.g. "\\server\mnt\"), but the same
> > issue applies to `tramp-drop-volume-letter', and this case should be
> > rare in practice anyway.
>
> Yep, we could handle this case afterwards. If needed.
>
> > - Jim
>
> Best regards, Michael.

[1] At least for sshx, my understanding is that this would only happen
if you had the connection info cached in ~/.emacs.d/tramp, so you
wouldn't see this with "emacs -Q" even with my patch to fix things. It
would matter in everyday usage though.

[2] https://git.savannah.gnu.org/cgit/emacs.git/tree/src/dired.c#n393

tramp-trace.log (4K) Download Attachment
0001-tramp.el-tramp-completion-file-name-regexp-default.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix hostname completion on MS Windows

Michael Albinus
Jim Porter <[hidden email]> writes:

Hi Jim,

>> I know, that Eli Zaretskii (maintainer of Emacs) has committed several
>> patches in order to drop such a volume letter for remote file names, but
>> I don't remember where they have been applied. Perhaps, this was an
>> Emacs 28 change.
>
> It looks like `file-name-completion' still adds the volume letter by
> calling `expand-file-name' on Emacs 28. I didn't see any changes to
> `file-name-completion' since Emacs 27.

Yes, you are right. I've pushed your patch to the repositories, with
slighly reformatted comment.

> [1] At least for sshx, my understanding is that this would only happen
> if you had the connection info cached in ~/.emacs.d/tramp, so you
> wouldn't see this with "emacs -Q" even with my patch to fix things. It
> would matter in everyday usage though.

Hmm, strange. Tramp knows several places to grab host names for
completion, for example ~/.ssh/config and ~/.ssh/known_hosts. Doesn't
this work for you?

Best regards, Michael.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix hostname completion on MS Windows

Michael Albinus
Michael Albinus <[hidden email]> writes:

Hi Jim,

>> [1] At least for sshx, my understanding is that this would only happen
>> if you had the connection info cached in ~/.emacs.d/tramp, so you
>> wouldn't see this with "emacs -Q" even with my patch to fix things. It
>> would matter in everyday usage though.
>
> Hmm, strange. Tramp knows several places to grab host names for
> completion, for example ~/.ssh/config and ~/.ssh/known_hosts. Doesn't
> this work for you?

Well, I've googled around, and it looks that the ssh server and client
directories on MS Windows are c:/ProgramData/ssh and
c:/Users/<user>/.ssh, see
https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_server_configuration
https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_keymanagement

Does the appended patch help for host name completion?

Best regards, Michael.


attachment0 (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix hostname completion on MS Windows

Jim Porter
On Fri, Apr 9, 2021 at 4:34 AM Michael Albinus <[hidden email]> wrote:

> > Hmm, strange. Tramp knows several places to grab host names for
> > completion, for example ~/.ssh/config and ~/.ssh/known_hosts. Doesn't
> > this work for you?
>
> Well, I've googled around, and it looks that the ssh server and client
> directories on MS Windows are c:/ProgramData/ssh and
> c:/Users/<user>/.ssh, see
> https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_server_configuration
> https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_keymanagement
>
> Does the appended patch help for host name completion?

The patch works correctly for me. Even without saved connection info
in ~/.emacs.d/tramp, host name completion works as expected (pulling
from .ssh/known_hosts in my case). I never even realized this was a
bug, since I was so used to getting host name completion from
~/.emacs.d/tramp!

> Best regards, Michael.

Thanks,
Jim

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix hostname completion on MS Windows

Jim Porter
In reply to this post by Michael Albinus
On Fri, Apr 9, 2021 at 1:03 AM Michael Albinus <[hidden email]> wrote:
>
> Jim Porter <[hidden email]> writes:
> > It looks like `file-name-completion' still adds the volume letter by
> > calling `expand-file-name' on Emacs 28. I didn't see any changes to
> > `file-name-completion' since Emacs 27.
>
> Yes, you are right. I've pushed your patch to the repositories, with
> slighly reformatted comment.

I noticed that I neglected to include this change for the "simplified"
and "separate" syntaxes (I'd forgotten they exist, since I just use
the default syntax). Attached is a patch for those.

- Jim

0001-tramp.el-tramp-completion-file-name-regexp-simplifie.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix hostname completion on MS Windows

Michael Albinus
Jim Porter <[hidden email]> writes:

Hi Jim,

> I noticed that I neglected to include this change for the "simplified"
> and "separate" syntaxes (I'd forgotten they exist, since I just use
> the default syntax). Attached is a patch for those.

Last night, in bed, I had exactly the same idea :-)

I've pushed this to the repositories. Both patches together exceed the
small patch line limit a little bit, but I believe it is acceptable,
because at least the comments repeat themselves.

I shouldn't be a pedantic German :-)

> - Jim

Best regards, Michael.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix hostname completion on MS Windows

Michael Albinus
In reply to this post by Jim Porter
Jim Porter <[hidden email]> writes:

Hi Jim,

> The patch works correctly for me. Even without saved connection info
> in ~/.emacs.d/tramp, host name completion works as expected (pulling
> from .ssh/known_hosts in my case). I never even realized this was a
> bug, since I was so used to getting host name completion from
> ~/.emacs.d/tramp!

I've pushed a modified version of the patch to the repositories.

> Thanks,
> Jim

Best regards, Michael.