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

Previous Next

Full log

Message #40 received at 51622 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 51622 <at> debbugs.gnu.org
Subject: Re: bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories
 in `abbreviate-file-name'
Date: Sat, 13 Nov 2021 18:10:50 -0800

[Message part 1 (text/plain, inline)]

On 11/7/2021 10:37 AM, Michael Albinus wrote:
> Nice. I've pushed them to the emacs-28 branch in your name, merged also
> to the master branch. Maybe you could also add a test for quoted file
> names, i.e. a file name "/:/home/albinus/" should *not* be
> abbreviated. See files-tests-file-name-non-special-* tests in
> files-tests.el.

Ok, I added a test for this in the first patch.

>> +** Tramp
>> +
>> ++++
>> +*** Tramp supports abbreviating remote home directories now.
>> +When calling 'abbreviate-file-name' on a Tramp filename, the result
>> +will abbreviate the home directory to "~".
> 
> You made it under the Tramp heading. I believe there shall be a more
> general entry, that 'abbreviate-file-name' respects file name handlers
> now. The entry in the Tramp section can be kept, but in a more general
> sense. We don't abbreviate only to "~", but also to "~user", see below.

I added a NEWS entry to mention that `abbreviate-file-name' respects 
file name handlers now. I didn't update the Tramp entry though since I 
haven't added "~user" support (at least, not yet...). See below for some 
explanation.

> Tramp can more. If there are two remote users "jim" and "micha", then
> you have
> 
> (expand-file-name "/ssh:jim <at> host:~/") => "/ssh:jim <at> host:/home/jim/"
> (expand-file-name "/ssh:jim <at> host:~micha/") => "/ssh:jim <at> host:/home/micha/"
> 
> abbreviate-file-name is somehow the reverse function of
> expand-file-name. So it would be great, if we could have
> 
> (abbreviate-file-name "/ssh:jim <at> host:/home/micha/") => "/ssh:jim <at> host:~micha/"
> 
> Of course, we cannot check for any remote user's home directory. But we
> have the Tramp cache. expand-file-name adds connection properties for
> home directories, like ("~" . "/home/jim") and ("~micha" . "/home/micha")
> in the above examples. If somebody has already used
> "/ssh:jim <at> host:~micha/", and this is cached as ("~micha" . "/home/micha"),
> then abbreviate-file-name shall do such an abbreviation. WDYT?

I looked at doing this, but it was a bit more complex than I thought it 
would be initially, so I've held off for now. This does seem like a 
useful feature though, and would probably be nice to have for local 
paths too. It should be possible to enhance `expand-file-name' to cache 
the "~user" -> "/home/user" mapping similarly to how 
`tramp-sh-handle-expand-file-name' does.

I could keep working on this patch to add "~user" support, or maybe that 
could be a followup for later. Incidentally, another interesting feature 
would be abbreviating default methods/users. That's probably easy when 
Tramp has filled in those values since the file name has `tramp-default' 
properties set. I'm not sure how tricky it would be to do without those 
properties though.

>> +    ;; If any elt of directory-abbrev-alist matches this name or the
>> +    ;; home dir, abbreviate accordingly.
>> +    (dolist (dir-abbrev directory-abbrev-alist)
>> +      (when (string-match (car dir-abbrev) filename)
>> +        (setq filename
>> +              (concat (cdr dir-abbrev)
>> +                      (substring filename (match-end 0)))))
>> +      (when (string-match (car dir-abbrev) home-dir)
>> +        (setq home-dir
>> +              (concat (cdr dir-abbrev)
>> +                      (substring home-dir (match-end 0))))))
> 
> Well, this is copied code from abbreviate-file-name. Usually I try to
> avoid such code duplication, it's hard to maintain when it changes in
> the first place . Instead, I call the original function via
> tramp-run-real-handler, and use the result for further changes.
> 
> But abbreviate-file-name does much more than handling
> directory-abbrev-alist. Hmm.

I've tried to reduce as much duplication as possible here by creating 
two new functions: `directory-abbrev-make-regexp' and 
`directory-abbrev-apply'. The former just takes a directory and returns 
a regexp that would match it, and the latter loops over 
`directory-abbrev-alist' and applies the transformations.

I tried to do this via `(tramp-run-real-handler #'abbreviate-file-name 
...)', but it ended up being simpler (and faster to execute) this way.

> I prefer to keep things together. So I would like to see
> tramp-testNN-abbreviate-file-name closed to
> tramp-test05-expand-file-name. Could we call the new function
> tramp-test07-abbreviate-file-name? I know it will be a lot of work to
> rename all the other test functions, and I herewith volunteer to do this
> dirty task.

Ok, fixed.

>> +  (let ((home-dir (expand-file-name "/mock:localhost:~")))
> 
> You hard-code the mock method. But this is available only if
> $REMOTE_TEMPORARY_FILE_DIRECTORY is not set; I'd prefer to run
> tramp-tests.el in many different environments. So please use something
> like
> 
> (let ((home-dir (expand-file-name (concat (file-remote-p tramp-test-temporary-file-directory) "~")))))

Fixed this too.

I also attached a slightly-updated benchmark script as well as new 
results. Performance on local file names is the same as before the 
patch, and just slightly faster than before with Tramp file names. (Most 
of the performance improvements here happened in bug#51699, so I mainly 
wanted to maintain the current performance in this patch.)

[0001-Add-another-abbreviate-file-name-test.patch (text/plain, attachment)]

[0002-Support-abbreviating-home-directory-of-Tramp-filenam.patch (text/plain, attachment)]

[benchmark.el (text/plain, attachment)]

[benchmark-results.txt (text/plain, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.