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@host:~/") => "/ssh:jim@host:/home/jim/" > (expand-file-name "/ssh:jim@host:~micha/") => "/ssh:jim@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@host:/home/micha/") => "/ssh:jim@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@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.)