bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name'

[Michael Albinus]

Jim Porter <jporterbugs@gmail.com> writes:

Hi Jim,

> Currently, `abbreviate-file-name' abbreviates home directories, but
> only for the local system. For example:
>
>    $ emacs -Q
>    M-: (abbreviate-file-name "/home/jim/src") RET
>    ;;  => "~/src"
>    M-: (abbreviate-file-name "/sshx:localhost:/home/jim/src") RET
>    ;;  => "/sshx:localhost:/home/jim/src"
>
> It'd be nice to abbreviate TRAMP home dirs, especially for the buffer
> list, where the long length of TRAMP paths means that I often just see
> the same leading bits of the paths repeated in the File column. As a
> result, it can be hard to tell the exact file it refers to. (Of
> course, as a workaround, I could just widen the window.)

Nice idea :-)

> Attached is a patch series to do this, but the patches probably
> warrant some explanation. First, I removed `automount-dir-prefix';
> it's been obsolete since 24.3, and it would have made implementation
> of the second part more complex.

This is not related to the problem, and it isn't needed because I
propose to skip your second patch (see below). Personally I have no
opinion about, you might apply this patch if you believe it makes sense,
but independently from the feature request.

> Second, I removed the caching of the abbreviated home dir. Since
> adding TRAMP support means there are multiple home dirs (one per
> host), keeping the caching would have been fairly complex, and it's
> already the source of potential bugs (e.g. when temporarily setting
> HOME to something else). I did some benchmarking on this (see
> attached), and while it is indeed slower without the caching, I don't
> think it's worth keeping the caching around. The real performance cost
> comes from calling `abbreviate-file-name' with a TRAMP file (even
> before my patch), but abbreviating a local file is quite fast, even
> with a pathologically large `directory-abbrev-alist'. I also wrote a
> couple of unit tests to make sure this function works correctly.

I disagree. We shall keep the cached abbreviated-home-dir as *local*
home directory. Remote home directories shall be handled in Tramp, and
nowhere else.

This is a general design goal which I try to follow. Mixing Tramp needs
with other packages is good for trouble, and shall be avoided if possible.

> Finally, I added the actual TRAMP support. This has a pretty
> significant    performance hit to TRAMP files. Looking at profiles,
> this appears to be because my patch calls both
> `file-name-case-insensitive-p' and `file-remote-p' on the TRAMP path,
> and these duplicate quite a bit of work. Is there a way to make this
> more efficient (e.g. by getting the file handler just once instead of
> twice)? It might also be useful to add some unit tests here, but I
> wasn't 100% sure how to do that with TRAMP paths (the tests in my
> benchmark actually open an SSH connection, so that probably won't work
> on all systems).

I believe there is a much simpler solution: Add the following entry
(derived from your example) to directory-abbrev-alist:

("\\`/sshx:localhost:/home/jim" . "/sshx:localhost:~")

The appended patch is a proof of concept w/o any systematic testing, it
is not ready for commit. You might use it in order to get the idea, and
to provide an applicable patch. It handles only tramp-sh.el; other Tramp
backends might need something similar.

Tramp tests could be added to tramp-tests.el. You'll see there a Tramp
mockup method which gives you the possibility to add a test w/o a
working ssh connection or alike.

> In addition to the patches, I've also attached a simple benchmark
> script that I used to measure the performance of these patches as well
> as the results from my system. The performance for local paths is
> still quite good I think, and even the worst-case scenario for TRAMP
> paths (abbreviating with a 500-item `directory-abbrev-alist') clocks
> in at 4.6ms per call. It'd be nice to make that faster, but maybe
> that's the best we can do if we want this feature.

I'm eager to see new figures with an adapted patch.

Best regards, Michael.

txtocrsTZLPwC.txt
Description: Text Data