bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbre


From: Jim Porter
Subject: bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name'
Date: Sat, 6 Nov 2021 09:38:59 -0700

On 11/6/2021 8:34 AM, Michael Albinus wrote:
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.

Ok, I can do that. I could even add caching for remote hosts if people think it would help (it would probably improve performance, at least a little bit). However, while I was looking at the implementation of `abbreviate-file-name', I saw the following comment:

      ;; FIXME Is it even worth caching abbreviated-home-dir?
      ;; Ref: https://debbugs.gnu.org/19657#20

After looking over the explanation in that link, I decided to see what the performance impact would be if I removed the caching. In my opinion, the benchmarks suggest that the caching a small enough impact that the brittleness with using the cache outweighed the benefits. However, I don't feel strongly about that and if the cache should stay, that's ok with me.

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:~")

I had thought about doing that originally, but when I looked into the implementation to understand why home-dir abbreviation didn't work on remote files, I figured the better long-term solution is to fix `abbreviate-file-name' somehow. Then everyone benefits from the improvement.

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.

Is it a general rule that all Tramp-specific stuff goes into the Tramp files, or would it be ok to write a patch that only touches files.el, so long as the performance for local files isn't hurt? It wouldn't be terribly difficult to replace `abbreviated-home-dir' with something that handles multiple hosts, similar to `grep-host-defaults-alist'.

If I understand things correctly, `file-remote-p' can be non-nil for a non-Tramp file if it has a file name handler that says so (e.g. if I wrote my own package that handles some remote files in a different way from Tramp). Maybe that's an argument in favor of changing this in `abbreviate-file-name'. Then it works with any remote file. On the other hand, maybe we need more protocol-specific information to do this correctly, and I should do this inside Tramp...

Either way, I'll look at your patch and see how it compares; if doing it that way ends up being better, then I'll try to implement something like that. I see in your patch that you add to `directory-abbrev-alist'. Is it ok to change a defcustom automatically like this? It seems to work in my limited tests, but I thought defcustoms were for users to set themselves. Should I come up with a different way to do this if I want to merge it into Emacs?

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.

Thanks, I'll take a look.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]