* bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name'
@ 2021-11-06 3:44 Jim Porter
2021-11-06 8:06 ` Eli Zaretskii
2021-11-06 15:34 ` Michael Albinus
0 siblings, 2 replies; 18+ messages in thread
From: Jim Porter @ 2021-11-06 3:44 UTC (permalink / raw)
To: 51622
[-- Attachment #1: Type: text/plain, Size: 2707 bytes --]
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.)
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.
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.
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).
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.
[-- Attachment #2: 0001-Remove-automount-dir-prefix.patch --]
[-- Type: text/plain, Size: 2836 bytes --]
From 1b6d76d60929a10e13fbfe1d9b4286bbd0aebb58 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 4 Nov 2021 20:14:00 -0700
Subject: [PATCH 1/3] Remove 'automount-dir-prefix'
* lisp/file.el (automount-dir-prefix): Remove it.
* etc/NEWS: Mention the removal.
---
etc/NEWS | 5 +++++
lisp/files.el | 15 +--------------
2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index 211d943a14..ee96876ff9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -363,6 +363,11 @@ with recent versions of Firefox.
** The function 'image-dired-get-exif-data' is now obsolete.
Use 'exif-parse-file' and 'exif-field' instead.
+---
+** The obsolete variable 'automount-dir-prefix' has been removed.
+In Emacs 24.3, the variable 'automount-dir-prefix' was made obsolete.
+For similar functionality, use 'directory-abbrev-alist' instead.
+
\f
* Lisp Changes in Emacs 29.1
diff --git a/lisp/files.el b/lisp/files.el
index 173198a424..995f0cf97a 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1990,12 +1990,6 @@ create-file-buffer
(concat "|" lastname)
lastname))))
-(defcustom automount-dir-prefix (purecopy "^/tmp_mnt/")
- "Regexp to match the automounter prefix in a directory name."
- :group 'files
- :type 'regexp)
-(make-obsolete-variable 'automount-dir-prefix 'directory-abbrev-alist "24.3")
-
(defvar abbreviated-home-dir nil
"Regexp matching the user's homedir at the beginning of file name.
The value includes abbreviation according to `directory-abbrev-alist'.")
@@ -2003,21 +1997,14 @@ abbreviated-home-dir
(defun abbreviate-file-name (filename)
"Return a version of FILENAME shortened using `directory-abbrev-alist'.
This also substitutes \"~\" for the user's home directory (unless the
-home directory is a root directory) and removes automounter prefixes
-\(see the variable `automount-dir-prefix').
+home directory is a root directory).
When this function is first called, it caches the user's home
directory as a regexp in `abbreviated-home-dir', and reuses it
afterwards (so long as the home directory does not change;
if you want to permanently change your home directory after having
started Emacs, set `abbreviated-home-dir' to nil so it will be recalculated)."
- ;; Get rid of the prefixes added by the automounter.
(save-match-data ;FIXME: Why?
- (if (and automount-dir-prefix
- (string-match automount-dir-prefix filename)
- (file-exists-p (file-name-directory
- (substring filename (1- (match-end 0))))))
- (setq filename (substring filename (1- (match-end 0)))))
;; Avoid treating /home/foo as /home/Foo during `~' substitution.
(let ((case-fold-search (file-name-case-insensitive-p filename)))
;; If any elt of directory-abbrev-alist matches this name,
--
2.25.1
[-- Attachment #3: 0002-Don-t-cache-abbreviated-homedir-for-abbreviate-file-.patch --]
[-- Type: text/plain, Size: 14019 bytes --]
From cdca6502cc08dfea9b7c728606ab29b84129f23a Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 5 Nov 2021 19:25:26 -0700
Subject: [PATCH 2/3] Don't cache abbreviated homedir for
'abbreviate-file-name'
This is error-prone and the performance difference is minor,
especially when compared to the cost of abbreviating a TRAMP filename.
* lisp/files.el (abbreviated-home-dir): Mark as obsolete.
(abbreviate-file-name): Don't cache the abbreviated home directory.
* test/lisp/files-tests.el (files-tests-abbreviate-file-name-homedir):
(files-tests-abbreviate-file-name-directory-abbrev-alist):
New functions.
(files-tests-abbreviated-home-dir): Removed.
* lisp/startup.el (command-line):
* test/lisp/calendar/todo-mode-tests.el (with-todo-test):
* test/lisp/emacs-lisp/package-tests.el (with-package-test):
* test/lisp/progmodes/flymake-tests.el (ruby-backend):
Don't clear 'abbreviated-home-dir'.
* etc/NEWS: Announce the obsoletion of 'abbreviated-home-dir'.
---
etc/NEWS | 5 ++
lisp/files.el | 108 +++++++++++---------------
lisp/startup.el | 3 -
test/lisp/calendar/todo-mode-tests.el | 3 -
test/lisp/emacs-lisp/package-tests.el | 1 -
test/lisp/files-tests.el | 36 ++++++---
test/lisp/progmodes/flymake-tests.el | 5 +-
7 files changed, 76 insertions(+), 85 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index ee96876ff9..e0dda1c2aa 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -363,6 +363,11 @@ with recent versions of Firefox.
** The function 'image-dired-get-exif-data' is now obsolete.
Use 'exif-parse-file' and 'exif-field' instead.
+---
+** The variable 'abbreviated-home-dir' is now obsolete.
+'abbreviate-file-name' no longer caches the abbreviated home
+directory, so this variable isn't needed anymore.
+
---
** The obsolete variable 'automount-dir-prefix' has been removed.
In Emacs 24.3, the variable 'automount-dir-prefix' was made obsolete.
diff --git a/lisp/files.el b/lisp/files.el
index 995f0cf97a..94b78df40c 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1993,79 +1993,59 @@ create-file-buffer
(defvar abbreviated-home-dir nil
"Regexp matching the user's homedir at the beginning of file name.
The value includes abbreviation according to `directory-abbrev-alist'.")
+(make-obsolete-variable 'abbreviated-home-dir 'nil "29.1")
(defun abbreviate-file-name (filename)
"Return a version of FILENAME shortened using `directory-abbrev-alist'.
This also substitutes \"~\" for the user's home directory (unless the
-home directory is a root directory).
-
-When this function is first called, it caches the user's home
-directory as a regexp in `abbreviated-home-dir', and reuses it
-afterwards (so long as the home directory does not change;
-if you want to permanently change your home directory after having
-started Emacs, set `abbreviated-home-dir' to nil so it will be recalculated)."
+home directory is a root directory)."
(save-match-data ;FIXME: Why?
;; Avoid treating /home/foo as /home/Foo during `~' substitution.
- (let ((case-fold-search (file-name-case-insensitive-p filename)))
+ (let ((case-fold-search (file-name-case-insensitive-p filename))
+ (home-dir (expand-file-name "~")))
;; If any elt of directory-abbrev-alist matches this name,
;; abbreviate accordingly.
(dolist (dir-abbrev directory-abbrev-alist)
- (if (string-match (car dir-abbrev) filename)
- (setq filename
- (concat (cdr dir-abbrev)
- (substring filename (match-end 0))))))
- ;; Compute and save the abbreviated homedir name.
- ;; We defer computing this until the first time it's needed, to
- ;; give time for directory-abbrev-alist to be set properly.
- ;; We include a slash at the end, to avoid spurious matches
- ;; such as `/usr/foobar' when the home dir is `/usr/foo'.
- (unless abbreviated-home-dir
- (put 'abbreviated-home-dir 'home (expand-file-name "~"))
- (setq abbreviated-home-dir
- (let* ((abbreviated-home-dir "\\`\\'.") ;Impossible regexp.
- (regexp
- (concat "\\`"
- (regexp-quote
- (abbreviate-file-name
- (get 'abbreviated-home-dir 'home)))
- "\\(/\\|\\'\\)")))
- ;; Depending on whether default-directory does or
- ;; doesn't include non-ASCII characters, the value
- ;; of abbreviated-home-dir could be multibyte or
- ;; unibyte. In the latter case, we need to decode
- ;; it. Note that this function is called for the
- ;; first time (from startup.el) when
- ;; locale-coding-system is already set up.
- (if (multibyte-string-p regexp)
- regexp
- (decode-coding-string regexp
- (if (eq system-type 'windows-nt)
- 'utf-8
- locale-coding-system))))))
-
- ;; If FILENAME starts with the abbreviated homedir,
- ;; and ~ hasn't changed since abbreviated-home-dir was set,
- ;; make it start with `~' instead.
- ;; If ~ has changed, we ignore abbreviated-home-dir rather than
- ;; invalidating it, on the assumption that a change in HOME
- ;; is likely temporary (eg for testing).
- ;; FIXME Is it even worth caching abbreviated-home-dir?
- ;; Ref: https://debbugs.gnu.org/19657#20
- (let (mb1)
- (if (and (string-match abbreviated-home-dir filename)
- (setq mb1 (match-beginning 1))
- ;; If the home dir is just /, don't change it.
- (not (and (= (match-end 0) 1)
- (= (aref filename 0) ?/)))
- ;; MS-DOS root directories can come with a drive letter;
- ;; Novell Netware allows drive letters beyond `Z:'.
- (not (and (memq system-type '(ms-dos windows-nt cygwin))
- (string-match "\\`[a-zA-`]:/\\'" filename)))
- (equal (get 'abbreviated-home-dir 'home)
- (expand-file-name "~")))
- (setq filename
- (concat "~"
- (substring filename mb1))))
+ (when (string-match (car dir-abbrev) filename)
+ (setq filename (concat (cdr dir-abbrev)
+ (substring filename (match-end 0)))))
+ ;; Abbreviate the home directory too so that it can match the
+ ;; filename.
+ (when (string-match (car dir-abbrev) home-dir)
+ (setq home-dir (concat (cdr dir-abbrev)
+ (substring home-dir (match-end 0))))))
+ ;; If FILENAME starts with the abbreviated homedir make it start
+ ;; with `~' instead.
+ (if-let* ((home-dir-regexp
+ ;; We include a slash at the end, to avoid spurious
+ ;; matches such as `/usr/foobar' when the home dir is
+ ;; `/usr/foo'.
+ (concat "\\`" (regexp-quote home-dir) "\\(/\\|\\'\\)"))
+ (home-dir-regexp
+ ;; Depending on whether default-directory does or
+ ;; doesn't include non-ASCII characters, the value of
+ ;; home-dir-regexp could be multibyte or unibyte. In
+ ;; the latter case, we need to decode it. Note that
+ ;; this function is called for the first time (from
+ ;; startup.el) when locale-coding-system is already
+ ;; set up.
+ (if (multibyte-string-p home-dir-regexp)
+ home-dir-regexp
+ (decode-coding-string home-dir-regexp
+ (if (eq system-type 'windows-nt)
+ 'utf-8
+ locale-coding-system))))
+ ((string-match home-dir-regexp filename))
+ (mb1 (match-beginning 1))
+ ((and
+ ;; If the home dir is just /, don't change it.
+ (not (and (= (match-end 0) 1)
+ (= (aref filename 0) ?/)))
+ ;; MS-DOS root directories can come with a drive letter;
+ ;; Novell Netware allows drive letters beyond `Z:'.
+ (not (and (memq system-type '(ms-dos windows-nt cygwin))
+ (string-match "\\`[a-zA-`]:/\\'" filename))))))
+ (concat "~" (substring filename mb1))
filename))))
(defun find-buffer-visiting (filename &optional predicate)
diff --git a/lisp/startup.el b/lisp/startup.el
index 505d7b83f4..6400088c35 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -1050,9 +1050,6 @@ command-line
after-init-time nil
command-line-default-directory default-directory)
- ;; Force recomputation, in case it was computed during the dump.
- (setq abbreviated-home-dir nil)
-
;; See if we should import version-control from the environment variable.
(let ((vc (getenv "VERSION_CONTROL")))
(cond ((eq vc nil)) ;don't do anything if not set
diff --git a/test/lisp/calendar/todo-mode-tests.el b/test/lisp/calendar/todo-mode-tests.el
index 9b5d990b9b..dd8c5ec353 100644
--- a/test/lisp/calendar/todo-mode-tests.el
+++ b/test/lisp/calendar/todo-mode-tests.el
@@ -38,9 +38,6 @@ with-todo-test
"Set up an isolated `todo-mode' test environment."
(declare (debug (body)))
`(let* ((todo-test-home (make-temp-file "todo-test-home-" t))
- ;; Since we change HOME, clear this to avoid a conflict
- ;; e.g. if Emacs runs within the user's home directory.
- (abbreviated-home-dir nil)
(process-environment (cons (format "HOME=%s" todo-test-home)
process-environment))
(todo-directory (ert-resource-directory))
diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el
index 1fd93bc1be..e8651a1182 100644
--- a/test/lisp/emacs-lisp/package-tests.el
+++ b/test/lisp/emacs-lisp/package-tests.el
@@ -122,7 +122,6 @@ with-package-test
(package-gnupghome-dir (expand-file-name "gnupg" package-user-dir))
(package-archives `(("gnu" . ,(or ,location package-test-data-dir))))
(default-directory package-test-file-dir)
- abbreviated-home-dir
package--initialized
package-alist
,@(if update-news
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 4b9d4e4516..6015120cf6 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1342,19 +1342,35 @@ files-tests-copy-directory
(should (file-directory-p (concat (file-name-as-directory dest2) "a")))
(delete-directory dir 'recursive)))
-(ert-deftest files-tests-abbreviated-home-dir ()
- "Test that changing HOME does not confuse `abbreviate-file-name'.
-See <https://debbugs.gnu.org/19657#20>."
+(ert-deftest files-tests-abbreviate-file-name-homedir ()
+ ;; Check homedir abbreviation.
(let* ((homedir temporary-file-directory)
(process-environment (cons (format "HOME=%s" homedir)
- process-environment))
- (abbreviated-home-dir nil)
- (testfile (expand-file-name "foo" homedir))
- (old (file-truename (abbreviate-file-name testfile)))
- (process-environment (cons (format "HOME=%s"
- (expand-file-name "bar" homedir))
process-environment)))
- (should (equal old (file-truename (abbreviate-file-name testfile))))))
+ (should (equal "~/foo/bar"
+ (abbreviate-file-name (concat homedir "foo/bar")))))
+ ;; Check that homedir abbreviation doesn't occur when homedir is just /.
+ (let* ((homedir "/")
+ (process-environment (cons (format "HOME=%s" homedir)
+ process-environment)))
+ (should (equal "/foo/bar"
+ (abbreviate-file-name (concat homedir "foo/bar"))))))
+
+(ert-deftest files-tests-abbreviate-file-name-directory-abbrev-alist ()
+ ;; Check `directory-abbrev-alist' abbreviation.
+ (let ((directory-abbrev-alist '(("\\`/nowhere/special" . "/nw/sp"))))
+ (should (equal "/nw/sp/here"
+ (abbreviate-file-name "/nowhere/special/here"))))
+ ;; Check homedir and `directory-abbrev-alist' abbreviation.
+ (let* ((homedir temporary-file-directory)
+ (process-environment (cons (format "HOME=%s" homedir)
+ process-environment))
+ (directory-abbrev-alist
+ `((,(concat "\\`" (regexp-quote homedir) "nowhere/special")
+ . ,(concat homedir "nw/sp")))))
+ (should (equal "~/nw/sp/here"
+ (abbreviate-file-name
+ (concat homedir "nowhere/special/here"))))))
(ert-deftest files-tests-executable-find ()
"Test that `executable-find' works also with a relative or remote PATH.
diff --git a/test/lisp/progmodes/flymake-tests.el b/test/lisp/progmodes/flymake-tests.el
index 4c0d15d1e1..9d378f54ba 100644
--- a/test/lisp/progmodes/flymake-tests.el
+++ b/test/lisp/progmodes/flymake-tests.el
@@ -125,10 +125,7 @@ ruby-backend
;; Some versions of ruby fail if HOME doesn't exist (bug#29187).
(let* ((tempdir (make-temp-file "flymake-tests-ruby" t))
(process-environment (cons (format "HOME=%s" tempdir)
- process-environment))
- ;; And see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19657#20
- ;; for this particular yuckiness
- (abbreviated-home-dir nil))
+ process-environment)))
(unwind-protect
(let ((ruby-mode-hook
(lambda ()
--
2.25.1
[-- Attachment #4: 0003-Abbreviate-home-directory-for-remote-files.patch --]
[-- Type: text/plain, Size: 2105 bytes --]
From 0a4fb38555646226dc45ed6c86e3bae2d108bf6b Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 5 Nov 2021 20:11:04 -0700
Subject: [PATCH 3/3] Abbreviate home directory for remote files
* lisp/files.el (abbreviate-file-name): Support homedir abbreviation
of remote files.
---
lisp/files.el | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/lisp/files.el b/lisp/files.el
index 94b78df40c..aa230dc0c6 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2001,8 +2001,9 @@ abbreviate-file-name
home directory is a root directory)."
(save-match-data ;FIXME: Why?
;; Avoid treating /home/foo as /home/Foo during `~' substitution.
- (let ((case-fold-search (file-name-case-insensitive-p filename))
- (home-dir (expand-file-name "~")))
+ (let* ((case-fold-search (file-name-case-insensitive-p filename))
+ (remote-host (file-remote-p filename))
+ (home-dir (expand-file-name (concat remote-host "~"))))
;; If any elt of directory-abbrev-alist matches this name,
;; abbreviate accordingly.
(dolist (dir-abbrev directory-abbrev-alist)
@@ -2039,13 +2040,13 @@ abbreviate-file-name
(mb1 (match-beginning 1))
((and
;; If the home dir is just /, don't change it.
- (not (and (= (match-end 0) 1)
- (= (aref filename 0) ?/)))
+ (not (and (= (match-end 0) (1+ (length remote-host)))
+ (= (aref filename (length remote-host)) ?/)))
;; MS-DOS root directories can come with a drive letter;
;; Novell Netware allows drive letters beyond `Z:'.
(not (and (memq system-type '(ms-dos windows-nt cygwin))
(string-match "\\`[a-zA-`]:/\\'" filename))))))
- (concat "~" (substring filename mb1))
+ (concat remote-host "~" (substring filename mb1))
filename))))
(defun find-buffer-visiting (filename &optional predicate)
--
2.25.1
[-- Attachment #5: benchmark.el --]
[-- Type: text/plain, Size: 1237 bytes --]
(setq remote-host "/sshx:jim@localhost:")
(defun fill-directory-abbrev-alist (count)
(setq directory-abbrev-alist
(let (result)
(dotimes (i count result)
(setq result (cons (cons (format "\\`/home/abbr%d" (1+ i))
(format "/home/abbr%d" i))
result))))))
(defun run-test (count &optional path)
(let* ((abbreviate-home-dir nil)
(path (or path "/home/user/src/project"))
(remote-path (concat remote-host path)))
(benchmark 1000 `(abbreviate-file-name ,path))
(benchmark 1000 `(abbreviate-file-name ,remote-path))))
(find-file (concat remote-host "~"))
(message "Empty `directory-abbrev-alist'")
(run-test 1000)
(message "")
(fill-directory-abbrev-alist 100)
(message "100 items in `directory-abbrev-alist' (no matches)")
(run-test 1000)
(message "")
(message "100 items in `directory-abbrev-alist' (all matches)")
(run-test 1000 "/home/abbr100/src/project")
(message "")
(fill-directory-abbrev-alist 500)
(message "500 items in `directory-abbrev-alist' (no matches)")
(run-test 1000)
(message "")
(message "500 items in `directory-abbrev-alist' (all matches)")
(run-test 1000 "/home/abbr100/src/project")
[-- Attachment #6: benchmark-results.txt --]
[-- Type: text/plain, Size: 2612 bytes --]
(Note: each test uses 1000 iterations of abbreviate-file-name.)
Vanilla Emacs 29.0.50:
----------------------
Empty ‘directory-abbrev-alist’
Local | Elapsed time: 0.051336s (0.016125s in 1 GCs)
TRAMP | Elapsed time: 2.173621s (0.501653s in 38 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.316021s (0.133531s in 10 GCs)
TRAMP | Elapsed time: 2.459212s (0.633373s in 48 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.539636s (0.327637s in 25 GCs)
TRAMP | Elapsed time: 2.484629s (0.642597s in 49 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.075650s (0.500043s in 38 GCs)
TRAMP | Elapsed time: 3.241414s (1.006143s in 76 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.302201s (0.696249s in 53 GCs)
TRAMP | Elapsed time: 3.261984s (1.018098s in 77 GCs)
Remove caching (patches 1 and 2):
---------------------------------
Empty ‘directory-abbrev-alist’
Local | Elapsed time: 0.075684s (0.012997s in 1 GCs)
TRAMP | Elapsed time: 2.196654s (0.514075s in 39 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.361488s (0.130064s in 10 GCs)
TRAMP | Elapsed time: 2.517825s (0.641286s in 49 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.616476s (0.340155s in 26 GCs)
TRAMP | Elapsed time: 2.534926s (0.641485s in 49 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.239399s (0.502237s in 38 GCs)
TRAMP | Elapsed time: 3.394109s (1.012237s in 77 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.472967s (0.707569s in 54 GCs)
TRAMP | Elapsed time: 3.401517s (1.008865s in 77 GCs)
TRAMP support (patches 1-3):
----------------------------
Local | Elapsed time: 0.076620s (0.013434s in 1 GCs)
TRAMP | Elapsed time: 3.543266s (0.864729s in 64 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.365402s (0.131290s in 10 GCs)
TRAMP | Elapsed time: 3.736696s (0.955474s in 73 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.608122s (0.340677s in 26 GCs)
TRAMP | Elapsed time: 3.818867s (0.956361s in 73 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.232221s (0.499099s in 38 GCs)
TRAMP | Elapsed time: 4.599799s (1.313629s in 100 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.479609s (0.708903s in 54 GCs)
TRAMP | Elapsed time: 4.656963s (1.345125s in 101 GCs)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-06 3:44 bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name' Jim Porter
@ 2021-11-06 8:06 ` Eli Zaretskii
2021-11-06 15:34 ` Michael Albinus
1 sibling, 0 replies; 18+ messages in thread
From: Eli Zaretskii @ 2021-11-06 8:06 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Fri, 5 Nov 2021 20:44:44 -0700
>
> 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.
If some user never uses Tramp, this removal will be a net loss for
that user. So I think we should have this customizable. Or maybe
even better: as long as Tramp is not used, keep the cache, and only
stop updating it when Tramp is being used.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-06 3:44 bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name' Jim Porter
2021-11-06 8:06 ` Eli Zaretskii
@ 2021-11-06 15:34 ` Michael Albinus
2021-11-06 16:38 ` Jim Porter
1 sibling, 1 reply; 18+ messages in thread
From: Michael Albinus @ 2021-11-06 15:34 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622
[-- Attachment #1: Type: text/plain, Size: 4068 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 768 bytes --]
diff --git a/lisp/tramp-sh.el b/lisp/tramp-sh.el
index 6f3b3245..974e3f3a 100644
--- a/lisp/tramp-sh.el
+++ b/lisp/tramp-sh.el
@@ -2723,6 +2723,14 @@ the result will be a local, non-Tramp, file name."
(format "cd %s && pwd" (tramp-shell-quote-argument uname)))
(with-current-buffer (tramp-get-buffer v)
(goto-char (point-min))
+ (add-to-list
+ 'directory-abbrev-alist
+ (cons
+ (concat
+ "\\`" (tramp-make-tramp-file-name v 'noloc 'nohop)
+ (buffer-substring (point) (point-at-eol)))
+ (concat
+ (tramp-make-tramp-file-name v 'noloc 'nohop) uname)))
(buffer-substring (point) (point-at-eol)))))
(setq localname (concat uname fname))))
;; There might be a double slash, for example when "~/"
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-06 15:34 ` Michael Albinus
@ 2021-11-06 16:38 ` Jim Porter
2021-11-06 17:41 ` Michael Albinus
0 siblings, 1 reply; 18+ messages in thread
From: Jim Porter @ 2021-11-06 16:38 UTC (permalink / raw)
To: Michael Albinus; +Cc: 51622
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-06 16:38 ` Jim Porter
@ 2021-11-06 17:41 ` Michael Albinus
2021-11-07 3:30 ` bug#51622: 29.0.50; [PATCH v2] " Jim Porter
0 siblings, 1 reply; 18+ messages in thread
From: Michael Albinus @ 2021-11-06 17:41 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622
Jim Porter <jporterbugs@gmail.com> writes:
Hi Jim,
>> 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.
As said, I have no opinion about this. However, we shall care Eli's
opinion, who dislikes such a change.
>> 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...
The general rule is, to keep all this specific handling away from
files.el and companions. Emacs has the concept of file name handlers,
and if there is something to be done special, a file name handler shall
offer an alternative implementation. This is true for remote file names
(packages tramp*.el, ange-ftp.el, url-handler.el) as well as for other
special handling like automatic compression/decompression of files
(jka-compr.el), GPG encryption/decryption (epa*.el), handling image
files (mage-file.el), handling of tar files (tar-mode.el) etc. Not every
file name handler must implement all "magic" functions, if there's no
specific implementation, the original function is used.
See (info "(elisp) Magic File Names") for the list of basic functions a
file name handler could offer its own implementation, currently these
are 75 functions.
> 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?
If we want to implement a general purpose solution, not only for remote
file names in Tramp, we shall add `abbreviate-file-name' to that
list. Different file name handlers could start to offer their own
implementation, for the other file name handlers the current default
behavior will be kept. Perhaps we shall go this way.
And the first candidate would be tramp-sh.el.
Best regards, Michael.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-06 17:41 ` Michael Albinus
@ 2021-11-07 3:30 ` Jim Porter
2021-11-07 18:37 ` Michael Albinus
0 siblings, 1 reply; 18+ messages in thread
From: Jim Porter @ 2021-11-07 3:30 UTC (permalink / raw)
To: Michael Albinus; +Cc: 51622
[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]
On 11/6/2021 10:41 AM, Michael Albinus wrote:
> The general rule is, to keep all this specific handling away from
> files.el and companions. Emacs has the concept of file name handlers,
> and if there is something to be done special, a file name handler shall
> offer an alternative implementation.
Thanks for the pointers. I've attached a new version of the patch, along
with updated benchmark results. When abbreviating Tramp files, not only
is this version faster than my previous patch, it's also 2-4x faster(!)
than Emacs trunk.
I included a couple of related patches in this series, although I can
split them out if it would be easier. The first patch just reorders a
couple of Tramp tests that got added in the wrong order previously (I
only did this because I wanted to add my new test to the end, and
figured it would be simpler to fix the order first).
The second patch is the main patch and uses a file name handler as you
suggested. Hopefully I got everything right here; I'm not very familiar
with these parts of Tramp. The test I added passes for me, though a
bunch of the other Tramp tests fail for me (with or without my patches).
Finally, since I already had them lying around, I added a few tests for
`abbreviate-file-name' for local files. They're pretty simple, but
should help catch any regressions in the future.
[-- Attachment #2: 0001-test-lisp-net-tramp-tests.el-Rearrange-tests-to-be-i.patch --]
[-- Type: text/plain, Size: 5441 bytes --]
From 3038f1802e53affe96c243a0da11a9dc47e8c34c Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 6 Nov 2021 18:01:11 -0700
Subject: [PATCH 1/3] ; * test/lisp/net/tramp-tests.el: Rearrange tests to be
in order.
---
test/lisp/net/tramp-tests.el | 74 ++++++++++++++++++------------------
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 737e2209cc..3d6ce963ee 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -6999,8 +6999,40 @@ tramp-test44-asynchronous-requests
;; (tramp--test--deftest-direct-async-process tramp-test44-asynchronous-requests
;; "Check parallel direct asynchronous requests." 'unstable)
+(ert-deftest tramp-test45-dired-compress-file ()
+ "Check that Tramp (un)compresses normal files."
+ (skip-unless (tramp--test-enabled))
+ (skip-unless (tramp--test-sh-p))
+ (let ((default-directory tramp-test-temporary-file-directory)
+ (tmp-name (tramp--test-make-temp-name)))
+ (write-region "foo" nil tmp-name)
+ (dired default-directory)
+ (dired-revert)
+ (dired-goto-file tmp-name)
+ (should-not (dired-compress))
+ (should (string= (concat tmp-name ".gz") (dired-get-filename)))
+ (should-not (dired-compress))
+ (should (string= tmp-name (dired-get-filename)))
+ (delete-file tmp-name)))
+
+(ert-deftest tramp-test45-dired-compress-dir ()
+ "Check that Tramp (un)compresses directories."
+ (skip-unless (tramp--test-enabled))
+ (skip-unless (tramp--test-sh-p))
+ (let ((default-directory tramp-test-temporary-file-directory)
+ (tmp-name (tramp--test-make-temp-name)))
+ (make-directory tmp-name)
+ (dired default-directory)
+ (dired-revert)
+ (dired-goto-file tmp-name)
+ (should-not (dired-compress))
+ (should (string= (concat tmp-name ".tar.gz") (dired-get-filename)))
+ (should-not (dired-compress))
+ (should (string= tmp-name (dired-get-filename)))
+ (delete-directory tmp-name)))
+
;; This test is inspired by Bug#29163.
-(ert-deftest tramp-test45-auto-load ()
+(ert-deftest tramp-test46-auto-load ()
"Check that Tramp autoloads properly."
;; If we use another syntax but `default', Tramp is already loaded
;; due to the `tramp-change-syntax' call.
@@ -7025,7 +7057,7 @@ tramp-test45-auto-load
(mapconcat #'shell-quote-argument load-path " -L ")
(shell-quote-argument code)))))))
-(ert-deftest tramp-test45-delay-load ()
+(ert-deftest tramp-test46-delay-load ()
"Check that Tramp is loaded lazily, only when needed."
;; The autoloaded Tramp objects are different since Emacs 26.1. We
;; cannot test older Emacsen, therefore.
@@ -7058,7 +7090,7 @@ tramp-test45-delay-load
(mapconcat #'shell-quote-argument load-path " -L ")
(shell-quote-argument (format code tm)))))))))
-(ert-deftest tramp-test45-recursive-load ()
+(ert-deftest tramp-test46-recursive-load ()
"Check that Tramp does not fail due to recursive load."
(skip-unless (tramp--test-enabled))
@@ -7082,7 +7114,7 @@ tramp-test45-recursive-load
(mapconcat #'shell-quote-argument load-path " -L ")
(shell-quote-argument code))))))))
-(ert-deftest tramp-test45-remote-load-path ()
+(ert-deftest tramp-test46-remote-load-path ()
"Check that Tramp autoloads its packages with remote `load-path'."
;; The autoloaded Tramp objects are different since Emacs 26.1. We
;; cannot test older Emacsen, therefore.
@@ -7111,7 +7143,7 @@ tramp-test45-remote-load-path
(mapconcat #'shell-quote-argument load-path " -L ")
(shell-quote-argument code)))))))
-(ert-deftest tramp-test46-unload ()
+(ert-deftest tramp-test47-unload ()
"Check that Tramp and its subpackages unload completely.
Since it unloads Tramp, it shall be the last test to run."
:tags '(:expensive-test)
@@ -7169,38 +7201,6 @@ tramp-test46-unload
(ignore-errors (all-completions "tramp" (symbol-value x)))
(ert-fail (format "Hook `%s' still contains Tramp function" x))))))
-(ert-deftest tramp-test44-dired-compress-file ()
- "Check that Tramp (un)compresses normal files."
- (skip-unless (tramp--test-enabled))
- (skip-unless (tramp--test-sh-p))
- (let ((default-directory tramp-test-temporary-file-directory)
- (tmp-name (tramp--test-make-temp-name)))
- (write-region "foo" nil tmp-name)
- (dired default-directory)
- (dired-revert)
- (dired-goto-file tmp-name)
- (should-not (dired-compress))
- (should (string= (concat tmp-name ".gz") (dired-get-filename)))
- (should-not (dired-compress))
- (should (string= tmp-name (dired-get-filename)))
- (delete-file tmp-name)))
-
-(ert-deftest tramp-test44-dired-compress-dir ()
- "Check that Tramp (un)compresses directories."
- (skip-unless (tramp--test-enabled))
- (skip-unless (tramp--test-sh-p))
- (let ((default-directory tramp-test-temporary-file-directory)
- (tmp-name (tramp--test-make-temp-name)))
- (make-directory tmp-name)
- (dired default-directory)
- (dired-revert)
- (dired-goto-file tmp-name)
- (should-not (dired-compress))
- (should (string= (concat tmp-name ".tar.gz") (dired-get-filename)))
- (should-not (dired-compress))
- (should (string= tmp-name (dired-get-filename)))
- (delete-directory tmp-name)))
-
(defun tramp-test-all (&optional interactive)
"Run all tests for \\[tramp].
If INTERACTIVE is non-nil, the tests are run interactively."
--
2.25.1
[-- Attachment #3: 0002-Support-abbreviating-home-directory-of-Tramp-filenam.patch --]
[-- Type: text/plain, Size: 17783 bytes --]
From 1722432f502c62866eeb2f1b69f95bc3fe016c77 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 6 Nov 2021 17:19:29 -0700
Subject: [PATCH 2/3] Support abbreviating home directory of Tramp filenames
* lisp/files.el (abbreviate-file-name): Check for file name handler.
(file-name-non-special):
* lisp/net/tramp.el (tramp-file-name-for-operation):
* lisp/net/tramp-sh.el (tramp-sh-file-name-handler-alist):
Add 'abbreviate-file-name'.
(tramp-sh-handle-abbreviate-file-name): New function.
* test/lisp/net/tramp-tests.el (tramp-test46-abbreviate-file-name):
New test.
* doc/lispref/files.texi (Magic File Names): Mention
'abbreviate-file-name' in the list of magic file name handlers.
* etc/NEWS: Announce the change.
---
doc/lispref/files.texi | 7 +-
etc/NEWS | 7 ++
lisp/files.el | 139 ++++++++++++++++++-----------------
lisp/net/tramp-sh.el | 40 +++++++++-
lisp/net/tramp.el | 2 +
test/lisp/net/tramp-tests.el | 37 ++++++++--
6 files changed, 155 insertions(+), 77 deletions(-)
diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index ddc1d05c1c..9c50341b49 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -3295,8 +3295,8 @@ Magic File Names
@ifnottex
@noindent
-@code{access-file}, @code{add-name-to-file},
-@code{byte-compiler-base-file-name},@*
+@code{abbreviate-file-name}, @code{access-file},
+@code{add-name-to-file}, @code{byte-compiler-base-file-name},@*
@code{copy-directory}, @code{copy-file},
@code{delete-directory}, @code{delete-file},
@code{diff-latest-backup-file},
@@ -3355,7 +3355,8 @@ Magic File Names
@iftex
@noindent
@flushleft
-@code{access-file}, @code{add-name-to-file},
+@code{abbreviate-file-name}, @code{access-file},
+@code{add-name-to-file},
@code{byte-com@discretionary{}{}{}piler-base-file-name},
@code{copy-directory}, @code{copy-file},
@code{delete-directory}, @code{delete-file},
diff --git a/etc/NEWS b/etc/NEWS
index 78c848126a..07861ceee5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -361,6 +361,13 @@ the buffer will take you to that directory.
This is a convenience function to extract the field data from
'exif-parse-file' and 'exif-parse-buffer'.
+** 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 "~".
+
\f
* New Modes and Packages in Emacs 29.1
diff --git a/lisp/files.el b/lisp/files.el
index 3af9730326..bb88b3c524 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2013,73 +2013,75 @@ abbreviate-file-name
started Emacs, set `abbreviated-home-dir' to nil so it will be recalculated)."
;; Get rid of the prefixes added by the automounter.
(save-match-data ;FIXME: Why?
- (if (and automount-dir-prefix
- (string-match automount-dir-prefix filename)
- (file-exists-p (file-name-directory
- (substring filename (1- (match-end 0))))))
- (setq filename (substring filename (1- (match-end 0)))))
- ;; Avoid treating /home/foo as /home/Foo during `~' substitution.
- (let ((case-fold-search (file-name-case-insensitive-p filename)))
- ;; If any elt of directory-abbrev-alist matches this name,
- ;; abbreviate accordingly.
- (dolist (dir-abbrev directory-abbrev-alist)
- (if (string-match (car dir-abbrev) filename)
- (setq filename
- (concat (cdr dir-abbrev)
- (substring filename (match-end 0))))))
- ;; Compute and save the abbreviated homedir name.
- ;; We defer computing this until the first time it's needed, to
- ;; give time for directory-abbrev-alist to be set properly.
- ;; We include a slash at the end, to avoid spurious matches
- ;; such as `/usr/foobar' when the home dir is `/usr/foo'.
- (unless abbreviated-home-dir
- (put 'abbreviated-home-dir 'home (expand-file-name "~"))
- (setq abbreviated-home-dir
- (let* ((abbreviated-home-dir "\\`\\'.") ;Impossible regexp.
- (regexp
- (concat "\\`"
- (regexp-quote
- (abbreviate-file-name
- (get 'abbreviated-home-dir 'home)))
- "\\(/\\|\\'\\)")))
- ;; Depending on whether default-directory does or
- ;; doesn't include non-ASCII characters, the value
- ;; of abbreviated-home-dir could be multibyte or
- ;; unibyte. In the latter case, we need to decode
- ;; it. Note that this function is called for the
- ;; first time (from startup.el) when
- ;; locale-coding-system is already set up.
- (if (multibyte-string-p regexp)
- regexp
- (decode-coding-string regexp
- (if (eq system-type 'windows-nt)
- 'utf-8
- locale-coding-system))))))
-
- ;; If FILENAME starts with the abbreviated homedir,
- ;; and ~ hasn't changed since abbreviated-home-dir was set,
- ;; make it start with `~' instead.
- ;; If ~ has changed, we ignore abbreviated-home-dir rather than
- ;; invalidating it, on the assumption that a change in HOME
- ;; is likely temporary (eg for testing).
- ;; FIXME Is it even worth caching abbreviated-home-dir?
- ;; Ref: https://debbugs.gnu.org/19657#20
- (let (mb1)
- (if (and (string-match abbreviated-home-dir filename)
- (setq mb1 (match-beginning 1))
- ;; If the home dir is just /, don't change it.
- (not (and (= (match-end 0) 1)
- (= (aref filename 0) ?/)))
- ;; MS-DOS root directories can come with a drive letter;
- ;; Novell Netware allows drive letters beyond `Z:'.
- (not (and (memq system-type '(ms-dos windows-nt cygwin))
- (string-match "\\`[a-zA-`]:/\\'" filename)))
- (equal (get 'abbreviated-home-dir 'home)
- (expand-file-name "~")))
- (setq filename
- (concat "~"
- (substring filename mb1))))
- filename))))
+ (if-let* ((handler (find-file-name-handler filename 'abbreviate-file-name)))
+ (funcall handler 'abbreviate-file-name filename)
+ (if (and automount-dir-prefix
+ (string-match automount-dir-prefix filename)
+ (file-exists-p (file-name-directory
+ (substring filename (1- (match-end 0))))))
+ (setq filename (substring filename (1- (match-end 0)))))
+ ;; Avoid treating /home/foo as /home/Foo during `~' substitution.
+ (let ((case-fold-search (file-name-case-insensitive-p filename)))
+ ;; If any elt of directory-abbrev-alist matches this name,
+ ;; abbreviate accordingly.
+ (dolist (dir-abbrev directory-abbrev-alist)
+ (if (string-match (car dir-abbrev) filename)
+ (setq filename
+ (concat (cdr dir-abbrev)
+ (substring filename (match-end 0))))))
+ ;; Compute and save the abbreviated homedir name.
+ ;; We defer computing this until the first time it's needed, to
+ ;; give time for directory-abbrev-alist to be set properly.
+ ;; We include a slash at the end, to avoid spurious matches
+ ;; such as `/usr/foobar' when the home dir is `/usr/foo'.
+ (unless abbreviated-home-dir
+ (put 'abbreviated-home-dir 'home (expand-file-name "~"))
+ (setq abbreviated-home-dir
+ (let* ((abbreviated-home-dir "\\`\\'.") ;Impossible regexp.
+ (regexp
+ (concat "\\`"
+ (regexp-quote
+ (abbreviate-file-name
+ (get 'abbreviated-home-dir 'home)))
+ "\\(/\\|\\'\\)")))
+ ;; Depending on whether default-directory does or
+ ;; doesn't include non-ASCII characters, the value
+ ;; of abbreviated-home-dir could be multibyte or
+ ;; unibyte. In the latter case, we need to decode
+ ;; it. Note that this function is called for the
+ ;; first time (from startup.el) when
+ ;; locale-coding-system is already set up.
+ (if (multibyte-string-p regexp)
+ regexp
+ (decode-coding-string regexp
+ (if (eq system-type 'windows-nt)
+ 'utf-8
+ locale-coding-system))))))
+
+ ;; If FILENAME starts with the abbreviated homedir,
+ ;; and ~ hasn't changed since abbreviated-home-dir was set,
+ ;; make it start with `~' instead.
+ ;; If ~ has changed, we ignore abbreviated-home-dir rather than
+ ;; invalidating it, on the assumption that a change in HOME
+ ;; is likely temporary (eg for testing).
+ ;; FIXME Is it even worth caching abbreviated-home-dir?
+ ;; Ref: https://debbugs.gnu.org/19657#20
+ (let (mb1)
+ (if (and (string-match abbreviated-home-dir filename)
+ (setq mb1 (match-beginning 1))
+ ;; If the home dir is just /, don't change it.
+ (not (and (= (match-end 0) 1)
+ (= (aref filename 0) ?/)))
+ ;; MS-DOS root directories can come with a drive letter;
+ ;; Novell Netware allows drive letters beyond `Z:'.
+ (not (and (memq system-type '(ms-dos windows-nt cygwin))
+ (string-match "\\`[a-zA-`]:/\\'" filename)))
+ (equal (get 'abbreviated-home-dir 'home)
+ (expand-file-name "~")))
+ (setq filename
+ (concat "~"
+ (substring filename mb1))))
+ filename)))))
(defun find-buffer-visiting (filename &optional predicate)
"Return the buffer visiting file FILENAME (a string).
@@ -7811,10 +7813,11 @@ file-name-non-special
;; Get a list of the indices of the args that are file names.
(file-arg-indices
(cdr (or (assq operation
- '(;; The first seven are special because they
+ '(;; The first eight are special because they
;; return a file name. We want to include
;; the /: in the return value. So just
;; avoid stripping it in the first place.
+ (abbreviate-file-name)
(directory-file-name)
(expand-file-name)
(file-name-as-directory)
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 6292190940..1151cd2ae8 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -940,7 +940,8 @@ tramp-vc-registered-read-file-names
;; New handlers should be added here.
;;;###tramp-autoload
(defconst tramp-sh-file-name-handler-alist
- '((access-file . tramp-handle-access-file)
+ '((abbreviate-file-name . tramp-sh-handle-abbreviate-file-name)
+ (access-file . tramp-handle-access-file)
(add-name-to-file . tramp-sh-handle-add-name-to-file)
;; `byte-compiler-base-file-name' performed by default handler.
(copy-directory . tramp-sh-handle-copy-directory)
@@ -1798,6 +1799,43 @@ tramp-sh-handle-file-name-all-completions
(push (buffer-substring (point) (point-at-eol)) result)))
result))))))
+(defun tramp-sh-handle-abbreviate-file-name (filename)
+ "Like `abbreviate-file-name' for Tramp files."
+ (let (home-dir)
+ (with-parsed-tramp-file-name filename nil
+ (setq home-dir (tramp-sh-handle-expand-file-name
+ (tramp-make-tramp-file-name v "~"))))
+ ;; 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))))))
+ (let* ((home-dir-regexp
+ ;; We include a slash at the end, to avoid spurious
+ ;; matches such as `/usr/foobar' when the home dir is
+ ;; `/usr/foo'.
+ (concat "\\`" (regexp-quote home-dir) "\\(/\\|\\'\\)"))
+ (home-dir-regexp
+ ;; The value of home-dir-regexp could be multibyte or
+ ;; unibyte. In the latter case, we need to decode it.
+ (if (multibyte-string-p home-dir-regexp)
+ home-dir-regexp
+ (decode-coding-string home-dir-regexp
+ (if (eq system-type 'windows-nt)
+ 'utf-8
+ locale-coding-system)))))
+ (if (string-match home-dir-regexp filename)
+ (with-parsed-tramp-file-name filename nil
+ (tramp-make-tramp-file-name
+ v (concat "~" (substring filename (match-beginning 1)))))
+ filename))))
+
;; cp, mv and ln
(defun tramp-sh-handle-add-name-to-file
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index b152584c1f..740cb23ebe 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -2493,6 +2493,8 @@ tramp-file-name-for-operation
file-system-info
;; Emacs 28+ only.
file-locked-p lock-file make-lock-file-name unlock-file
+ ;; Emacs 29+ only.
+ abbreviate-file-name
;; Tramp internal magic file name function.
tramp-set-file-uid-gid))
(if (file-name-absolute-p (nth 0 args))
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 3d6ce963ee..5eea00c41e 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -6122,6 +6122,12 @@ tramp--test-emacs28-p
variables, so we check the Emacs version directly."
(>= emacs-major-version 28))
+(defun tramp--test-emacs29-p ()
+ "Check for Emacs version >= 29.1.
+Some semantics has been changed for there, w/o new functions or
+variables, so we check the Emacs version directly."
+ (>= emacs-major-version 29))
+
(defun tramp--test-adb-p ()
"Check, whether the remote host runs Android.
This requires restrictions of file name syntax."
@@ -7031,8 +7037,29 @@ tramp-test45-dired-compress-dir
(should (string= tmp-name (dired-get-filename)))
(delete-directory tmp-name)))
+(ert-deftest tramp-test46-abbreviate-file-name ()
+ "Check that Tramp abbreviates file names correctly."
+ (skip-unless (tramp--test-enabled))
+ (skip-unless (tramp--test-emacs29-p))
+
+ (let ((home-dir (expand-file-name "/mock:localhost:~")))
+ ;; Check home-dir abbreviation.
+ (should (equal (abbreviate-file-name (concat home-dir "/foo/bar"))
+ "/mock:localhost:~/foo/bar"))
+ (should (equal (abbreviate-file-name "/mock:localhost:/nowhere/special")
+ "/mock:localhost:/nowhere/special"))
+ ;; Check `directory-abbrev-alist' abbreviation.
+ (let ((directory-abbrev-alist
+ `((,(concat "\\`" (regexp-quote home-dir) "/foo")
+ . ,(concat home-dir "/f"))
+ ("\\`/mock:localhost:/nowhere" . "/mock:localhost:/nw"))))
+ (should (equal (abbreviate-file-name (concat home-dir "/foo/bar"))
+ "/mock:localhost:~/f/bar"))
+ (should (equal (abbreviate-file-name "/mock:localhost:/nowhere/special")
+ "/mock:localhost:/nw/special")))))
+
;; This test is inspired by Bug#29163.
-(ert-deftest tramp-test46-auto-load ()
+(ert-deftest tramp-test47-auto-load ()
"Check that Tramp autoloads properly."
;; If we use another syntax but `default', Tramp is already loaded
;; due to the `tramp-change-syntax' call.
@@ -7057,7 +7084,7 @@ tramp-test46-auto-load
(mapconcat #'shell-quote-argument load-path " -L ")
(shell-quote-argument code)))))))
-(ert-deftest tramp-test46-delay-load ()
+(ert-deftest tramp-test47-delay-load ()
"Check that Tramp is loaded lazily, only when needed."
;; The autoloaded Tramp objects are different since Emacs 26.1. We
;; cannot test older Emacsen, therefore.
@@ -7090,7 +7117,7 @@ tramp-test46-delay-load
(mapconcat #'shell-quote-argument load-path " -L ")
(shell-quote-argument (format code tm)))))))))
-(ert-deftest tramp-test46-recursive-load ()
+(ert-deftest tramp-test47-recursive-load ()
"Check that Tramp does not fail due to recursive load."
(skip-unless (tramp--test-enabled))
@@ -7114,7 +7141,7 @@ tramp-test46-recursive-load
(mapconcat #'shell-quote-argument load-path " -L ")
(shell-quote-argument code))))))))
-(ert-deftest tramp-test46-remote-load-path ()
+(ert-deftest tramp-test47-remote-load-path ()
"Check that Tramp autoloads its packages with remote `load-path'."
;; The autoloaded Tramp objects are different since Emacs 26.1. We
;; cannot test older Emacsen, therefore.
@@ -7143,7 +7170,7 @@ tramp-test46-remote-load-path
(mapconcat #'shell-quote-argument load-path " -L ")
(shell-quote-argument code)))))))
-(ert-deftest tramp-test47-unload ()
+(ert-deftest tramp-test48-unload ()
"Check that Tramp and its subpackages unload completely.
Since it unloads Tramp, it shall be the last test to run."
:tags '(:expensive-test)
--
2.25.1
[-- Attachment #4: 0003-Add-some-unit-tests-for-abbreviate-file-name.patch --]
[-- Type: text/plain, Size: 2634 bytes --]
From f9da68bd2ad777a7ef345a8668b1fce284e597da Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 6 Nov 2021 20:15:56 -0700
Subject: [PATCH 3/3] Add some unit tests for 'abbreviate-file-name'
* test/lisp/files-tests.el (files-tests-abbreviate-file-name-homedir):
(files-tests-abbreviate-file-name-directory-abbrev-alist):
New tests.
---
test/lisp/files-tests.el | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 4b9d4e4516..1aea8af54f 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1342,6 +1342,39 @@ files-tests-copy-directory
(should (file-directory-p (concat (file-name-as-directory dest2) "a")))
(delete-directory dir 'recursive)))
+(ert-deftest files-tests-abbreviate-file-name-homedir ()
+ ;; Check homedir abbreviation.
+ (let* ((homedir temporary-file-directory)
+ (process-environment (cons (format "HOME=%s" homedir)
+ process-environment))
+ (abbreviated-home-dir nil))
+ (should (equal "~/foo/bar"
+ (abbreviate-file-name (concat homedir "foo/bar")))))
+ ;; Check that homedir abbreviation doesn't occur when homedir is just /.
+ (let* ((homedir "/")
+ (process-environment (cons (format "HOME=%s" homedir)
+ process-environment))
+ (abbreviated-home-dir nil))
+ (should (equal "/foo/bar"
+ (abbreviate-file-name (concat homedir "foo/bar"))))))
+
+(ert-deftest files-tests-abbreviate-file-name-directory-abbrev-alist ()
+ ;; Check `directory-abbrev-alist' abbreviation.
+ (let ((directory-abbrev-alist '(("\\`/nowhere/special" . "/nw/sp"))))
+ (should (equal "/nw/sp/here"
+ (abbreviate-file-name "/nowhere/special/here"))))
+ ;; Check homedir and `directory-abbrev-alist' abbreviation.
+ (let* ((homedir temporary-file-directory)
+ (process-environment (cons (format "HOME=%s" homedir)
+ process-environment))
+ (abbreviated-home-dir nil)
+ (directory-abbrev-alist
+ `((,(concat "\\`" (regexp-quote homedir) "nowhere/special")
+ . ,(concat homedir "nw/sp")))))
+ (should (equal "~/nw/sp/here"
+ (abbreviate-file-name
+ (concat homedir "nowhere/special/here"))))))
+
(ert-deftest files-tests-abbreviated-home-dir ()
"Test that changing HOME does not confuse `abbreviate-file-name'.
See <https://debbugs.gnu.org/19657#20>."
--
2.25.1
[-- Attachment #5: benchmark-results.txt --]
[-- Type: text/plain, Size: 2553 bytes --]
(Note: each test uses 1000 iterations of abbreviate-file-name.)
Vanilla Emacs 29.0.50:
----------------------
Empty ‘directory-abbrev-alist’
Local | Elapsed time: 0.051336s (0.016125s in 1 GCs)
TRAMP | Elapsed time: 2.173621s (0.501653s in 38 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.316021s (0.133531s in 10 GCs)
TRAMP | Elapsed time: 2.459212s (0.633373s in 48 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.539636s (0.327637s in 25 GCs)
TRAMP | Elapsed time: 2.484629s (0.642597s in 49 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.075650s (0.500043s in 38 GCs)
TRAMP | Elapsed time: 3.241414s (1.006143s in 76 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.302201s (0.696249s in 53 GCs)
TRAMP | Elapsed time: 3.261984s (1.018098s in 77 GCs)
Updated Patches:
----------------
Empty ‘directory-abbrev-alist’
Local | Elapsed time: 0.050861s (0.013198s in 1 GCs)
TRAMP | Elapsed time: 0.505943s (0.128814s in 9 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.303332s (0.121235s in 9 GCs)
TRAMP | Elapsed time: 0.827096s (0.260078s in 19 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.543074s (0.331712s in 25 GCs)
TRAMP | Elapsed time: 0.823411s (0.253757s in 19 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.088234s (0.511505s in 38 GCs)
TRAMP | Elapsed time: 1.722948s (0.629359s in 47 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.306427s (0.701965s in 53 GCs)
TRAMP | Elapsed time: 1.759451s (0.653703s in 47 GCs)
Original Patches:
-----------------
Local | Elapsed time: 0.076620s (0.013434s in 1 GCs)
TRAMP | Elapsed time: 3.543266s (0.864729s in 64 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.365402s (0.131290s in 10 GCs)
TRAMP | Elapsed time: 3.736696s (0.955474s in 73 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.608122s (0.340677s in 26 GCs)
TRAMP | Elapsed time: 3.818867s (0.956361s in 73 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.232221s (0.499099s in 38 GCs)
TRAMP | Elapsed time: 4.599799s (1.313629s in 100 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.479609s (0.708903s in 54 GCs)
TRAMP | Elapsed time: 4.656963s (1.345125s in 101 GCs)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-07 3:30 ` bug#51622: 29.0.50; [PATCH v2] " Jim Porter
@ 2021-11-07 18:37 ` Michael Albinus
2021-11-08 4:54 ` Jim Porter
2021-11-14 2:10 ` Jim Porter
0 siblings, 2 replies; 18+ messages in thread
From: Michael Albinus @ 2021-11-07 18:37 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622
Jim Porter <jporterbugs@gmail.com> writes:
Hi Jim,
> Thanks for the pointers. I've attached a new version of the patch,
> along with updated benchmark results. When abbreviating Tramp files,
> not only is this version faster than my previous patch, it's also 2-4x
> faster(!) than Emacs trunk.
Thanks, it looks very promising. According to the benchmarks I'm not
surprised, because you use Tramp caches.
> I included a couple of related patches in this series, although I can
> split them out if it would be easier. The first patch just reorders a
> couple of Tramp tests that got added in the wrong order previously (I
> only did this because I wanted to add my new test to the end, and
> figured it would be simpler to fix the order first).
Thanks. I've kept that patch on hold for a while. During my illness, it
got applied, and so you did the dirty task to rearrange everything. I've
pushed it in your name to the master branch. Thanks.
> The second patch is the main patch and uses a file name handler as you
> suggested. Hopefully I got everything right here; I'm not very
> familiar with these parts of Tramp. The test I added passes for me,
> though a bunch of the other Tramp tests fail for me (with or without
> my patches).
Thanks, as said it looks promising. More detailed comments below. For
the other failing Tramp tests please report them. If you like as another
Emacs bug, or directly to me :-)
> Finally, since I already had them lying around, I added a few tests
> for `abbreviate-file-name' for local files. They're pretty simple, but
> should help catch any regressions in the future.
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.
> diff --git a/etc/NEWS b/etc/NEWS
> index 78c848126a..07861ceee5 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -361,6 +361,13 @@ the buffer will take you to that directory.
> This is a convenience function to extract the field data from
> 'exif-parse-file' and 'exif-parse-buffer'.
>
> +** 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.
> diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
> index 6292190940..1151cd2ae8 100644
> --- a/lisp/net/tramp-sh.el
> +++ b/lisp/net/tramp-sh.el
>
> +(defun tramp-sh-handle-abbreviate-file-name (filename)
> + "Like `abbreviate-file-name' for Tramp files."
> + (let (home-dir)
> + (with-parsed-tramp-file-name filename nil
> + (setq home-dir (tramp-sh-handle-expand-file-name
> + (tramp-make-tramp-file-name v "~"))))
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?
> + ;; 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.
> diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
> index 3d6ce963ee..5eea00c41e 100644
> --- a/test/lisp/net/tramp-tests.el
> +++ b/test/lisp/net/tramp-tests.el
> +(ert-deftest tramp-test46-abbreviate-file-name ()
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.
> + (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) "~")))))
Beside of these comments, I repeat myself: pretty good job! Thanks!
Best regards, Michael.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-07 18:37 ` Michael Albinus
@ 2021-11-08 4:54 ` Jim Porter
2021-11-08 15:58 ` Michael Albinus
2021-11-14 2:10 ` Jim Porter
1 sibling, 1 reply; 18+ messages in thread
From: Jim Porter @ 2021-11-08 4:54 UTC (permalink / raw)
To: Michael Albinus; +Cc: 51622
[-- Attachment #1: Type: text/plain, Size: 2475 bytes --]
On 11/7/2021 10:37 AM, Michael Albinus wrote:
> Jim Porter <jporterbugs@gmail.com> writes:
>
>> Thanks for the pointers. I've attached a new version of the patch,
>> along with updated benchmark results. When abbreviating Tramp files,
>> not only is this version faster than my previous patch, it's also 2-4x
>> faster(!) than Emacs trunk.
>
> Thanks, it looks very promising. According to the benchmarks I'm not
> surprised, because you use Tramp caches.
Hmm, actually it turns out that my patch was only this fast because I
forgot to check whether the host has case-sensitive file names or not.
Adding that check back in slows things down again. How I update my
previous patch will depend on whether we can make
`file-name-case-insensitive-p' fast for Tramp files, so I'll just focus
on this part for now and then follow up on the other parts of your
message after we've decided on what to do here.
Currently on case-sensitive hosts,
`tramp-handle-file-name-case-insensitive-p' performs its checks on the
connection every time this function is called. The beginning of tramp.el
says the following:
* `tramp-case-insensitive'
Whether the remote file system handles file names case insensitive.
Only a non-nil value counts, the default value nil means to
perform further checks on the remote host. See
`tramp-connection-properties' for a way to overwrite this.
I interpret this to mean that Tramp *intentionally* performs checks on
the host every time if the result is nil. Is there a reason this is
necessary? Are there any systems out there where the check would return
nil, but it's still case-insensitive in some cases? Even if there are, I
imagine that *most* of the time, this check is reliable, and it would be
nice if we could cache the result for case-sensitive hosts.
I've attached the beginnings of a patch to do this. What do you think?
If the general idea makes sense, I'll finish it up and file a separate
bug to track it. If Tramp needs to perform the checks every time for
some remote hosts, maybe the user could set `tramp-case-insensitive' to
`never-cache' for those connections?
> Thanks. I've kept that patch on hold for a while. During my illness, it
> got applied, and so you did the dirty task to rearrange everything. I've
> pushed it in your name to the master branch. Thanks.
I hope your health is doing better now. Thanks again for taking a look
at this patch (and merging the two smaller ones).
[-- Attachment #2: tramp-cache-case-sensitive.patch --]
[-- Type: text/plain, Size: 5432 bytes --]
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 740cb23ebe..9393cfe589 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -1469,21 +1469,22 @@ tramp-file-name-equal-p
(equal (tramp-file-name-unify vec1)
(tramp-file-name-unify vec2))))
-(defun tramp-get-method-parameter (vec param)
+(defun tramp-get-method-parameter (vec param &optional default)
"Return the method parameter PARAM.
If VEC is a vector, check first in connection properties.
Afterwards, check in `tramp-methods'. If the `tramp-methods'
-entry does not exist, return nil."
+entry does not exist, return DEFAULT."
(let ((hash-entry
(replace-regexp-in-string "^tramp-" "" (symbol-name param))))
(if (tramp-connection-property-p vec hash-entry)
;; We use the cached property.
- (tramp-get-connection-property vec hash-entry nil)
+ (tramp-get-connection-property vec hash-entry default)
;; Use the static value from `tramp-methods'.
- (when-let ((methods-entry
- (assoc
- param (assoc (tramp-file-name-method vec) tramp-methods))))
- (cadr methods-entry)))))
+ (if-let ((methods-entry
+ (assoc
+ param (assoc (tramp-file-name-method vec) tramp-methods))))
+ (cadr methods-entry)
+ default))))
;; The localname can be quoted with "/:". Extract this.
(defun tramp-file-name-unquote-localname (vec)
@@ -3482,51 +3483,57 @@ tramp-handle-file-name-case-insensitive-p
;; tests will be added in case they are needed.
(setq filename (expand-file-name filename))
(with-parsed-tramp-file-name filename nil
- (or ;; Maybe there is a default value.
- (tramp-get-method-parameter v 'tramp-case-insensitive)
-
- ;; There isn't. So we must check, in case there's a connection already.
- (and (file-remote-p filename nil 'connected)
- (with-tramp-connection-property v "case-insensitive"
- (ignore-errors
- (with-tramp-progress-reporter v 5 "Checking case-insensitive"
- ;; The idea is to compare a file with lower case
- ;; letters with the same file with upper case letters.
- (let ((candidate
- (tramp-compat-file-name-unquote
- (directory-file-name filename)))
- case-fold-search
- tmpfile)
- ;; Check, whether we find an existing file with
- ;; lower case letters. This avoids us to create a
- ;; temporary file.
- (while (and (string-match-p
- "[[:lower:]]" (tramp-file-local-name candidate))
- (not (file-exists-p candidate)))
- (setq candidate
- (directory-file-name
- (file-name-directory candidate))))
- ;; Nothing found, so we must use a temporary file
- ;; for comparison. `make-nearby-temp-file' is added
- ;; to Emacs 26+ like `file-name-case-insensitive-p',
- ;; so there is no compatibility problem calling it.
- (unless (string-match-p
- "[[:lower:]]" (tramp-file-local-name candidate))
- (setq tmpfile
- (let ((default-directory
- (file-name-directory filename)))
- (tramp-compat-funcall
- 'make-nearby-temp-file "tramp."))
- candidate tmpfile))
- ;; Check for the existence of the same file with
- ;; upper case letters.
- (unwind-protect
- (file-exists-p
- (concat
- (file-remote-p candidate)
- (upcase (tramp-file-local-name candidate))))
- ;; Cleanup.
- (when tmpfile (delete-file tmpfile)))))))))))
+ (let ((case-insensitive
+ (tramp-get-method-parameter v 'tramp-case-insensitive 'unset)))
+ (if (not (eq case-insensitive 'unset))
+ ;; Maybe there is a default value.
+ case-insensitive
+ ;; There isn't. So we must check, in case there's a
+ ;; connection already.
+ (and (file-remote-p filename nil 'connected)
+ (with-tramp-connection-property v "case-insensitive"
+ (ignore-errors
+ (with-tramp-progress-reporter v 5 "Checking case-insensitive"
+ ;; The idea is to compare a file with lower case
+ ;; letters with the same file with upper case
+ ;; letters.
+ (let ((candidate
+ (tramp-compat-file-name-unquote
+ (directory-file-name filename)))
+ case-fold-search
+ tmpfile)
+ ;; Check, whether we find an existing file with
+ ;; lower case letters. This avoids us to create
+ ;; a temporary file.
+ (while (and (string-match-p
+ "[[:lower:]]"
+ (tramp-file-local-name candidate))
+ (not (file-exists-p candidate)))
+ (setq candidate
+ (directory-file-name
+ (file-name-directory candidate))))
+ ;; Nothing found, so we must use a temporary file
+ ;; for comparison. `make-nearby-temp-file' is
+ ;; added to Emacs 26+ like
+ ;; `file-name-case-insensitive-p', so there is no
+ ;; compatibility problem calling it.
+ (unless (string-match-p
+ "[[:lower:]]" (tramp-file-local-name candidate))
+ (setq tmpfile
+ (let ((default-directory
+ (file-name-directory filename)))
+ (tramp-compat-funcall
+ 'make-nearby-temp-file "tramp."))
+ candidate tmpfile))
+ ;; Check for the existence of the same file with
+ ;; upper case letters.
+ (unwind-protect
+ (file-exists-p
+ (concat
+ (file-remote-p candidate)
+ (upcase (tramp-file-local-name candidate))))
+ ;; Cleanup.
+ (when tmpfile (delete-file tmpfile))))))))))))
(defun tramp-handle-file-name-completion
(filename directory &optional predicate)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-08 4:54 ` Jim Porter
@ 2021-11-08 15:58 ` Michael Albinus
2021-11-08 18:32 ` Jim Porter
0 siblings, 1 reply; 18+ messages in thread
From: Michael Albinus @ 2021-11-08 15:58 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622
Jim Porter <jporterbugs@gmail.com> writes:
Hi Jim,
> Hmm, actually it turns out that my patch was only this fast because I
> forgot to check whether the host has case-sensitive file names or
> not. Adding that check back in slows things down again. How I update
> my previous patch will depend on whether we can make
> `file-name-case-insensitive-p' fast for Tramp files, so I'll just
> focus on this part for now and then follow up on the other parts of
> your message after we've decided on what to do here.
>
> Currently on case-sensitive hosts,
> `tramp-handle-file-name-case-insensitive-p' performs its checks on the
> connection every time this function is called. The beginning of
> tramp.el says the following:
>
> * `tramp-case-insensitive'
> Whether the remote file system handles file names case insensitive.
> Only a non-nil value counts, the default value nil means to
> perform further checks on the remote host. See
> `tramp-connection-properties' for a way to overwrite this.
>
> I interpret this to mean that Tramp *intentionally* performs checks on
> the host every time if the result is nil.
No. It just means, that the value of `tramp-case-insensitive' in
`tramp-methods' is used only if it is non-nil. BUT there is also a check
for a connection property "case-insensitive", and that value is used if
it exists, be it nil or non-nil. Therefore, the whole check in
`tramp-handle-file-name-case-insensitive-p' is applied only once, and
afterwards the cached connection property is used.
See the saved connection properties in file ~/.emacs.d/tramp.
> Is there a reason this is necessary?
The data in `tramp-methods' are static. By default, only non-nil values
are relevant, and we shouldn't change this w/o any need, since we have
connection properties for dynamic data.
> Are there any systems out there where the check would return nil, but
> it's still case-insensitive in some cases? Even if there are, I
> imagine that *most* of the time, this check is reliable, and it would
> be nice if we could cache the result for case-sensitive hosts.
See the comment in `tramp-handle-file-name-case-insensitive-p':
--8<---------------cut here---------------start------------->8---
;; We make it a connection property, assuming that all file systems
;; on the remote host behave similar. This might be wrong for
;; mounted NFS directories or SMB/AFP shares; such more granular
;; tests will be added in case they are needed.
--8<---------------cut here---------------end--------------->8---
So we cache the result, even if it might be problematic. But honestly, I
haven't seen reports yet about such a problem.
> I've attached the beginnings of a patch to do this. What do you think?
> If the general idea makes sense, I'll finish it up and file a separate
> bug to track it. If Tramp needs to perform the checks every time for
> some remote hosts, maybe the user could set `tramp-case-insensitive'
> to `never-cache' for those connections?
Before we start such changes, we should understand your case. Why is it
computed every time? Why isn't the cached value used?
Can you debug? The cache for "case-insensitive" is read by
--8<---------------cut here---------------start------------->8---
(with-tramp-connection-property v "case-insensitive" ...
--8<---------------cut here---------------end--------------->8---
If there isn't a cached value, this macro evaluates its body, and sets
the cache.
You might set tramp-verbose to 7, and follow entries in the debug buffer like
--8<---------------cut here---------------start------------->8---
16:26:43.718831 tramp-get-connection-property (7) # case-insensitive undef; cache used: nil
16:26:43.729457 tramp-set-connection-property (7) # case-insensitive nil
16:26:43.734749 tramp-get-connection-property (7) # case-insensitive nil; cache used: t
--8<---------------cut here---------------end--------------->8---
> I hope your health is doing better now.
It will be up and down, as it happens for years. You will see me to
disappear for some days in the future, from time to time. That's (my) life.
Best regards, Michael.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-08 15:58 ` Michael Albinus
@ 2021-11-08 18:32 ` Jim Porter
2021-11-08 19:18 ` Michael Albinus
0 siblings, 1 reply; 18+ messages in thread
From: Jim Porter @ 2021-11-08 18:32 UTC (permalink / raw)
To: Michael Albinus; +Cc: 51622
On 11/8/2021 7:58 AM, Michael Albinus wrote:
> Jim Porter <jporterbugs@gmail.com> writes:
>
>> I've attached the beginnings of a patch to do this. What do you think?
>> If the general idea makes sense, I'll finish it up and file a separate
>> bug to track it. If Tramp needs to perform the checks every time for
>> some remote hosts, maybe the user could set `tramp-case-insensitive'
>> to `never-cache' for those connections?
>
> Before we start such changes, we should understand your case. Why is it
> computed every time? Why isn't the cached value used?
Thanks for the explanation above. It turns out I was just mistaken about
what's happening. The check on the remote host *is* performed only once.
Sorry for the confusion.
If I profile `tramp-handle-file-name-case-insensitive-p' on Emacs 29
master, I see that 52% of the time is spent in `file-remote-p' (and a
further 30% in `expand-file-name'). This is where the difference in
performance comes from; I don't think my patch helps with that, but when
testing, I eliminated the `file-remote-p' call (and the remote check)
and saw performance improve. I just got mixed up about what the issue was.
If the calls to `file-remote-p' and `expand-file-name' could be
optimized or replaced, that would make this function a lot faster. I'll
take a look at this and see what I can do to improve things. If you have
any suggestions though, I'm happy to hear about them.
(The main reason I'm digging into this right now is that you mentioned
it would be nice if we didn't have to copy-and-paste so much code from
`abbreviate-file-name'. I'm looking into making a new function called
something like `directory-abbrev-apply', which *only* does the
`directory-abbrev-alist' replacements so that Tramp can call this.
However, to make it easier to use correctly, I wanted to make sure
`case-fold-search' was set based on `file-name-case-insensitive-p'. I
only want to do that though if I can make that function fast enough that
it doesn't show up near the top of a performance profile. Otherwise,
I'll just try to keep the number of calls to
`file-name-case-insensitive-p' to the bare minimum.)
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-08 18:32 ` Jim Porter
@ 2021-11-08 19:18 ` Michael Albinus
0 siblings, 0 replies; 18+ messages in thread
From: Michael Albinus @ 2021-11-08 19:18 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622
Jim Porter <jporterbugs@gmail.com> writes:
Hi Jim,
> If I profile `tramp-handle-file-name-case-insensitive-p' on Emacs 29
> master, I see that 52% of the time is spent in `file-remote-p' (and a
> further 30% in `expand-file-name'). This is where the difference in
> performance comes from; I don't think my patch helps with that, but
> when testing, I eliminated the `file-remote-p' call (and the remote
> check) and saw performance improve. I just got mixed up about what the
> issue was.
>
> If the calls to `file-remote-p' and `expand-file-name' could be
> optimized or replaced, that would make this function a lot
> faster. I'll take a look at this and see what I can do to improve
> things. If you have any suggestions though, I'm happy to hear about
> them.
The check (file-remote-p filename nil 'connected) is applied to prevent
unintended work. It returns non-nil when there is already a connection
to the remote host. The intention is to avoid, that
`tramp-handle-file-name-case-insensitive-p' opens a new connection just
in order to see, whether the remote file system is case
insensitive. This is not needed when there's no connection yet.
Hmm, we could replace this check by something cheaper: is there already
a connection process?
--8<---------------cut here---------------start------------->8---
(and (let ((non-essential t)) (tramp-connectable-p filename))
...
--8<---------------cut here---------------end--------------->8---
The `non-essential' variable must be bound to non-nil; otherwise
`tramp-connectable-p' would always return non-nil.
Does this help? In case of, you might offer a patch, with a comment why
we don't use (file-remote-p filename nil 'connected) .
For the `expand-file-name' case I have no offer yet. But let's see how
the timing changes with that change above.
Btw, when you run benchmark checks, you shall set (or let-bind)
`tramp-verbose' to 0. Writing debug data has a cost.
> (The main reason I'm digging into this right now is that you mentioned
> it would be nice if we didn't have to copy-and-paste so much code from
> `abbreviate-file-name'. I'm looking into making a new function called
> something like `directory-abbrev-apply', which *only* does the
> `directory-abbrev-alist' replacements so that Tramp can call
> this.
Good idea, I was also thinking about. For backward compatibility we will
keep a mirror of that function in tramp-compat.el, until we support only
Emacs 29+. Isn't this a glory future? :-)
Best regards, Michael.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-07 18:37 ` Michael Albinus
2021-11-08 4:54 ` Jim Porter
@ 2021-11-14 2:10 ` Jim Porter
2021-11-14 14:43 ` Michael Albinus
1 sibling, 1 reply; 18+ messages in thread
From: Jim Porter @ 2021-11-14 2:10 UTC (permalink / raw)
To: Michael Albinus; +Cc: 51622
[-- Attachment #1: Type: text/plain, Size: 5209 bytes --]
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.)
[-- Attachment #2: 0001-Add-another-abbreviate-file-name-test.patch --]
[-- Type: text/plain, Size: 1376 bytes --]
From 003d012adc7671f9f0ad42cf9aca5f147a8d0c0c Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 13 Nov 2021 17:38:36 -0800
Subject: [PATCH 1/2] ; Add another 'abbreviate-file-name' test
* test/lisp/files-tests.el
(files-tests-abbreviate-file-name-non-special): New test.
---
test/lisp/files-tests.el | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index d00f1ce326..df57d78fca 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1364,6 +1364,15 @@ files-tests-abbreviate-file-name-directory-abbrev-alist
(abbreviate-file-name
(concat homedir "nowhere/special/here"))))))
+(ert-deftest files-tests-abbreviate-file-name-non-special ()
+ (let* ((homedir temporary-file-directory)
+ (process-environment (cons (format "HOME=%s" homedir)
+ process-environment))
+ (abbreviated-home-dir nil))
+ ;; Check that abbreviation doesn't occur for quoted file names.
+ (should (equal (concat "/:" homedir "foo/bar")
+ (abbreviate-file-name (concat "/:" homedir "foo/bar"))))))
+
(ert-deftest files-tests-abbreviated-home-dir ()
"Test that changing HOME does not confuse `abbreviate-file-name'.
See <https://debbugs.gnu.org/19657#20>."
--
2.25.1
[-- Attachment #3: 0002-Support-abbreviating-home-directory-of-Tramp-filenam.patch --]
[-- Type: text/plain, Size: 15607 bytes --]
From 5d049d9fb902819726b472782dd2d028e3d6165f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 13 Nov 2021 17:23:42 -0800
Subject: [PATCH 2/2] Support abbreviating home directory of Tramp filenames
* lisp/files.el (directory-abbrev-make-regexp):
(directory-abbrev-apply): New functions.
(abbreviate-file-name): Check for file name handler.
(file-name-non-special):
* lisp/net/tramp.el (tramp-file-name-for-operation):
* lisp/net/tramp-sh.el (tramp-sh-file-name-handler-alist):
Add 'abbreviate-file-name'.
(tramp-sh-handle-abbreviate-file-name): New function.
* test/lisp/net/tramp-tests.el (tramp-test46-abbreviate-file-name):
New test.
* doc/lispref/files.texi (Magic File Names): Mention
'abbreviate-file-name' in the list of magic file name handlers.
* etc/NEWS: Announce the change.
---
doc/lispref/files.texi | 7 +-
etc/NEWS | 10 +++
lisp/files.el | 143 ++++++++++++++++++-----------------
lisp/net/tramp-sh.el | 20 ++++-
lisp/net/tramp.el | 2 +
test/lisp/net/tramp-tests.el | 25 ++++++
6 files changed, 135 insertions(+), 72 deletions(-)
diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index d93770a0d2..4b114ba111 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -3308,8 +3308,8 @@ Magic File Names
@ifnottex
@noindent
-@code{access-file}, @code{add-name-to-file},
-@code{byte-compiler-base-file-name},@*
+@code{abbreviate-file-name}, @code{access-file},
+@code{add-name-to-file}, @code{byte-compiler-base-file-name},@*
@code{copy-directory}, @code{copy-file},
@code{delete-directory}, @code{delete-file},
@code{diff-latest-backup-file},
@@ -3368,7 +3368,8 @@ Magic File Names
@iftex
@noindent
@flushleft
-@code{access-file}, @code{add-name-to-file},
+@code{abbreviate-file-name}, @code{access-file},
+@code{add-name-to-file},
@code{byte-com@discretionary{}{}{}piler-base-file-name},
@code{copy-directory}, @code{copy-file},
@code{delete-directory}, @code{delete-file},
diff --git a/etc/NEWS b/etc/NEWS
index c362e56cee..48ff06ef28 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -497,6 +497,13 @@ The newly created buffer will be displayed via 'display-buffer', which
can be customized through the usual mechanism of 'display-buffer-alist'
and friends.
+** 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 "~".
+
\f
* New Modes and Packages in Emacs 29.1
@@ -632,6 +639,9 @@ This convenience function is useful when writing code that parses
files at run-time, and allows Lisp programs to re-parse files only
when they have changed.
++++
+** 'abbreviate-file-name' now respects magic file name handlers.
+
---
** New function 'font-has-char-p'.
This can be used to check whether a specific font has a glyph for a
diff --git a/lisp/files.el b/lisp/files.el
index 3490d0428a..49bf06bfc1 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -68,6 +68,31 @@ directory-abbrev-alist
:group 'abbrev
:group 'find-file)
+(defun directory-abbrev-make-regexp (directory)
+ "Create a regexp to match DIRECTORY for `directory-abbrev-alist'."
+ (let ((regexp
+ ;; We include a slash at the end, to avoid spurious
+ ;; matches such as `/usr/foobar' when the home dir is
+ ;; `/usr/foo'.
+ (concat "\\`" (regexp-quote directory) "\\(/\\|\\'\\)")))
+ ;; The value of regexp could be multibyte or unibyte. In the
+ ;; latter case, we need to decode it.
+ (if (multibyte-string-p regexp)
+ regexp
+ (decode-coding-string regexp
+ (if (eq system-type 'windows-nt)
+ 'utf-8
+ locale-coding-system)))))
+
+(defun directory-abbrev-apply (filename)
+ "Apply the abbreviations in `directory-abbrev-alist' to FILENAME.
+Note that when calling this, you should set `case-fold-search' as
+appropriate for the filesystem used for FILENAME."
+ (dolist (dir-abbrev directory-abbrev-alist filename)
+ (when (string-match (car dir-abbrev) filename)
+ (setq filename (concat (cdr dir-abbrev)
+ (substring filename (match-end 0)))))))
+
(defcustom make-backup-files t
"Non-nil means make a backup of a file the first time it is saved.
This can be done by renaming the file or by copying.
@@ -2015,73 +2040,54 @@ abbreviate-file-name
started Emacs, set `abbreviated-home-dir' to nil so it will be recalculated)."
;; Get rid of the prefixes added by the automounter.
(save-match-data ;FIXME: Why?
- (if (and automount-dir-prefix
- (string-match automount-dir-prefix filename)
- (file-exists-p (file-name-directory
- (substring filename (1- (match-end 0))))))
- (setq filename (substring filename (1- (match-end 0)))))
- ;; Avoid treating /home/foo as /home/Foo during `~' substitution.
- (let ((case-fold-search (file-name-case-insensitive-p filename)))
- ;; If any elt of directory-abbrev-alist matches this name,
- ;; abbreviate accordingly.
- (dolist (dir-abbrev directory-abbrev-alist)
- (if (string-match (car dir-abbrev) filename)
- (setq filename
- (concat (cdr dir-abbrev)
- (substring filename (match-end 0))))))
- ;; Compute and save the abbreviated homedir name.
- ;; We defer computing this until the first time it's needed, to
- ;; give time for directory-abbrev-alist to be set properly.
- ;; We include a slash at the end, to avoid spurious matches
- ;; such as `/usr/foobar' when the home dir is `/usr/foo'.
- (unless abbreviated-home-dir
- (put 'abbreviated-home-dir 'home (expand-file-name "~"))
- (setq abbreviated-home-dir
- (let* ((abbreviated-home-dir "\\`\\'.") ;Impossible regexp.
- (regexp
- (concat "\\`"
- (regexp-quote
- (abbreviate-file-name
- (get 'abbreviated-home-dir 'home)))
- "\\(/\\|\\'\\)")))
- ;; Depending on whether default-directory does or
- ;; doesn't include non-ASCII characters, the value
- ;; of abbreviated-home-dir could be multibyte or
- ;; unibyte. In the latter case, we need to decode
- ;; it. Note that this function is called for the
- ;; first time (from startup.el) when
- ;; locale-coding-system is already set up.
- (if (multibyte-string-p regexp)
- regexp
- (decode-coding-string regexp
- (if (eq system-type 'windows-nt)
- 'utf-8
- locale-coding-system))))))
-
- ;; If FILENAME starts with the abbreviated homedir,
- ;; and ~ hasn't changed since abbreviated-home-dir was set,
- ;; make it start with `~' instead.
- ;; If ~ has changed, we ignore abbreviated-home-dir rather than
- ;; invalidating it, on the assumption that a change in HOME
- ;; is likely temporary (eg for testing).
- ;; FIXME Is it even worth caching abbreviated-home-dir?
- ;; Ref: https://debbugs.gnu.org/19657#20
- (let (mb1)
- (if (and (string-match abbreviated-home-dir filename)
- (setq mb1 (match-beginning 1))
- ;; If the home dir is just /, don't change it.
- (not (and (= (match-end 0) 1)
- (= (aref filename 0) ?/)))
- ;; MS-DOS root directories can come with a drive letter;
- ;; Novell Netware allows drive letters beyond `Z:'.
- (not (and (memq system-type '(ms-dos windows-nt cygwin))
- (string-match "\\`[a-zA-`]:/\\'" filename)))
- (equal (get 'abbreviated-home-dir 'home)
- (expand-file-name "~")))
- (setq filename
- (concat "~"
- (substring filename mb1))))
- filename))))
+ (if-let ((handler (find-file-name-handler filename 'abbreviate-file-name)))
+ (funcall handler 'abbreviate-file-name filename)
+ (if (and automount-dir-prefix
+ (string-match automount-dir-prefix filename)
+ (file-exists-p (file-name-directory
+ (substring filename (1- (match-end 0))))))
+ (setq filename (substring filename (1- (match-end 0)))))
+ ;; Avoid treating /home/foo as /home/Foo during `~' substitution.
+ (let ((case-fold-search (file-name-case-insensitive-p filename)))
+ ;; If any elt of directory-abbrev-alist matches this name,
+ ;; abbreviate accordingly.
+ (setq filename (directory-abbrev-apply filename))
+
+ ;; Compute and save the abbreviated homedir name.
+ ;; We defer computing this until the first time it's needed, to
+ ;; give time for directory-abbrev-alist to be set properly.
+ (unless abbreviated-home-dir
+ (put 'abbreviated-home-dir 'home (expand-file-name "~"))
+ (setq abbreviated-home-dir
+ (directory-abbrev-make-regexp
+ (let ((abbreviated-home-dir "\\`\\'.")) ;Impossible regexp.
+ (abbreviate-file-name
+ (get 'abbreviated-home-dir 'home))))))
+
+ ;; If FILENAME starts with the abbreviated homedir,
+ ;; and ~ hasn't changed since abbreviated-home-dir was set,
+ ;; make it start with `~' instead.
+ ;; If ~ has changed, we ignore abbreviated-home-dir rather than
+ ;; invalidating it, on the assumption that a change in HOME
+ ;; is likely temporary (eg for testing).
+ ;; FIXME Is it even worth caching abbreviated-home-dir?
+ ;; Ref: https://debbugs.gnu.org/19657#20
+ (let (mb1)
+ (if (and (string-match abbreviated-home-dir filename)
+ (setq mb1 (match-beginning 1))
+ ;; If the home dir is just /, don't change it.
+ (not (and (= (match-end 0) 1)
+ (= (aref filename 0) ?/)))
+ ;; MS-DOS root directories can come with a drive letter;
+ ;; Novell Netware allows drive letters beyond `Z:'.
+ (not (and (memq system-type '(ms-dos windows-nt cygwin))
+ (string-match "\\`[a-zA-`]:/\\'" filename)))
+ (equal (get 'abbreviated-home-dir 'home)
+ (expand-file-name "~")))
+ (setq filename
+ (concat "~"
+ (substring filename mb1))))
+ filename)))))
(defun find-buffer-visiting (filename &optional predicate)
"Return the buffer visiting file FILENAME (a string).
@@ -7836,10 +7842,11 @@ file-name-non-special
;; Get a list of the indices of the args that are file names.
(file-arg-indices
(cdr (or (assq operation
- '(;; The first seven are special because they
+ '(;; The first eight are special because they
;; return a file name. We want to include
;; the /: in the return value. So just
;; avoid stripping it in the first place.
+ (abbreviate-file-name)
(directory-file-name)
(expand-file-name)
(file-name-as-directory)
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index c61025a86b..a77ffe432b 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -942,7 +942,8 @@ tramp-vc-registered-read-file-names
;; New handlers should be added here.
;;;###tramp-autoload
(defconst tramp-sh-file-name-handler-alist
- '((access-file . tramp-handle-access-file)
+ '((abbreviate-file-name . tramp-sh-handle-abbreviate-file-name)
+ (access-file . tramp-handle-access-file)
(add-name-to-file . tramp-sh-handle-add-name-to-file)
;; `byte-compiler-base-file-name' performed by default handler.
(copy-directory . tramp-sh-handle-copy-directory)
@@ -1801,6 +1802,23 @@ tramp-sh-handle-file-name-all-completions
(push (buffer-substring (point) (point-at-eol)) result)))
result))))))
+(defun tramp-sh-handle-abbreviate-file-name (filename)
+ "Like `abbreviate-file-name' for Tramp files."
+ (let* ((case-fold-search (tramp-handle-file-name-case-insensitive-p filename))
+ (home-dir
+ (with-parsed-tramp-file-name filename nil
+ (with-tramp-connection-property v "home-directory"
+ (directory-abbrev-apply (tramp-sh-handle-expand-file-name
+ (tramp-make-tramp-file-name v "~")))))))
+ ;; If any elt of directory-abbrev-alist matches this name,
+ ;; abbreviate accordingly.
+ (setq filename (directory-abbrev-apply filename))
+ (if (string-match (directory-abbrev-make-regexp home-dir) filename)
+ (with-parsed-tramp-file-name filename nil
+ (tramp-make-tramp-file-name
+ v (concat "~" (substring filename (match-beginning 1)))))
+ filename)))
+
;; cp, mv and ln
(defun tramp-sh-handle-add-name-to-file
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 876bbb2c54..52f39bf355 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -2495,6 +2495,8 @@ tramp-file-name-for-operation
file-system-info
;; Emacs 28+ only.
file-locked-p lock-file make-lock-file-name unlock-file
+ ;; Emacs 29+ only.
+ abbreviate-file-name
;; Tramp internal magic file name function.
tramp-set-file-uid-gid))
(if (file-name-absolute-p (nth 0 args))
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 52c6159dc1..698d18b528 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -2289,6 +2289,31 @@ tramp-test06-directory-file-name
(should (string-equal (file-name-directory file) file))
(should (string-equal (file-name-nondirectory file) "")))))))
+(ert-deftest tramp-test07-abbreviate-file-name ()
+ "Check that Tramp abbreviates file names correctly."
+ (skip-unless (tramp--test-enabled))
+ (skip-unless (tramp--test-emacs29-p))
+
+ (let* ((remote-host (file-remote-p tramp-test-temporary-file-directory))
+ (home-dir (expand-file-name (concat remote-host "~"))))
+ ;; Check home-dir abbreviation.
+ (should (equal (abbreviate-file-name (concat home-dir "/foo/bar"))
+ (concat remote-host "~/foo/bar")))
+ (should (equal (abbreviate-file-name (concat remote-host
+ "/nowhere/special"))
+ (concat remote-host "/nowhere/special")))
+ ;; Check `directory-abbrev-alist' abbreviation.
+ (let ((directory-abbrev-alist
+ `((,(concat "\\`" (regexp-quote home-dir) "/foo")
+ . ,(concat home-dir "/f"))
+ (,(concat "\\`" (regexp-quote remote-host) "/nowhere")
+ . ,(concat remote-host "/nw")))))
+ (should (equal (abbreviate-file-name (concat home-dir "/foo/bar"))
+ (concat remote-host "~/f/bar")))
+ (should (equal (abbreviate-file-name (concat remote-host
+ "/nowhere/special"))
+ (concat remote-host "/nw/special"))))))
+
(ert-deftest tramp-test07-file-exists-p ()
"Check `file-exist-p', `write-region' and `delete-file'."
(skip-unless (tramp--test-enabled))
--
2.25.1
[-- Attachment #4: benchmark.el --]
[-- Type: text/plain, Size: 1362 bytes --]
(setq tramp-verbose 0
user-name "jim"
remote-host (concat "/sshx:" user-name "@localhost:"))
(defun fill-directory-abbrev-alist (count)
(setq directory-abbrev-alist
(let (result)
(dotimes (i count result)
(setq result (cons (cons (format "\\`/home/abbr%d" (1+ i))
(format "/home/abbr%d" i))
result))))))
(defun run-test (count &optional path)
(let* ((abbreviate-home-dir nil)
(path (or path (concat "/home/" user-name "/src/project")))
(remote-path (concat remote-host path)))
(garbage-collect)
(benchmark 1000 `(abbreviate-file-name ,path))
(garbage-collect)
(benchmark 1000 `(abbreviate-file-name ,remote-path))))
(find-file (concat remote-host "~"))
(message "Empty `directory-abbrev-alist'")
(run-test 1000)
(message "")
(fill-directory-abbrev-alist 100)
(message "100 items in `directory-abbrev-alist' (no matches)")
(run-test 1000)
(message "")
(message "100 items in `directory-abbrev-alist' (all matches)")
(run-test 1000 "/home/abbr100/src/project")
(message "")
(fill-directory-abbrev-alist 500)
(message "500 items in `directory-abbrev-alist' (no matches)")
(run-test 1000)
(message "")
(message "500 items in `directory-abbrev-alist' (all matches)")
(run-test 1000 "/home/abbr100/src/project")
[-- Attachment #5: benchmark-results.txt --]
[-- Type: text/plain, Size: 1659 bytes --]
Emacs 29 master
---------------
Empty ‘directory-abbrev-alist’
Local | Elapsed time: 0.082094s (0.012981s in 1 GCs)
Tramp | Elapsed time: 0.570441s (0.175013s in 13 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.334320s (0.129351s in 10 GCs)
Tramp | Elapsed time: 0.838366s (0.294330s in 22 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.530187s (0.320165s in 25 GCs)
Tramp | Elapsed time: 0.836190s (0.285371s in 22 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.089229s (0.492225s in 38 GCs)
Tramp | Elapsed time: 1.587351s (0.649014s in 50 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.292103s (0.687631s in 53 GCs)
Tramp | Elapsed time: 1.592210s (0.651090s in 50 GCs)
With patch
----------
Empty ‘directory-abbrev-alist’
Local | Elapsed time: 0.076136s (0.012949s in 1 GCs)
Tramp | Elapsed time: 0.510445s (0.160052s in 12 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.342509s (0.130916s in 10 GCs)
Tramp | Elapsed time: 0.780201s (0.281118s in 21 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.538353s (0.323898s in 25 GCs)
Tramp | Elapsed time: 0.699262s (0.245942s in 19 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.104786s (0.500880s in 38 GCs)
Tramp | Elapsed time: 1.524662s (0.642236s in 49 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.299160s (0.687867s in 53 GCs)
Tramp | Elapsed time: 1.519829s (0.651691s in 47 GCs)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-14 2:10 ` Jim Porter
@ 2021-11-14 14:43 ` Michael Albinus
2021-11-15 6:58 ` bug#51622: 29.0.50; [PATCH v3] " Jim Porter
0 siblings, 1 reply; 18+ messages in thread
From: Michael Albinus @ 2021-11-14 14:43 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622
Jim Porter <jporterbugs@gmail.com> writes:
Hi Jim,
>> 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.
Thanks. However, I believe this test shall be called
`files-tests-file-name-non-special-file-abbreviate-file-name', like the
other non-special tests. And perhaps it shall be located prior
`files-tests-file-name-non-special-access-file'.
> 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.
Agreed.
> 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.
ATM, it might be sufficient to push what we have. Adding "~user" support
might come 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.
You cannot trust the `tramp-default' property. It is set when a method
or user or host name is expanded as in "/ssh::". But when the host name
is used explicitly by the user, as in "/ssh:host:", the property is not
set, even if "host" is the default. Same for user.
But it shouldn't be too hard to determine the defaults. We have
tramp-default-method{-alist}, tramp-default-user{-alist}, and
tramp-default-host{-alist}. All needed information is there.
> 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.
Fine, let's go this way. After your patch, we'll need some backward
compatibility voodoo in Tramp, but this can wait until the dust has settled.
> 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.)
Good, no regression :-)
Some few comments on the code:
> --- a/etc/NEWS
> +++ b/etc/NEWS
> +** Tramp
> +
> ++++
This shall be rather "---". We don't add documentation (yet) for this
new Tramp feature.
> +*** Tramp supports abbreviating remote home directories now.
> +When calling 'abbreviate-file-name' on a Tramp filename, the result
> +will abbreviate the home directory to "~".
This might be misleading. ... the result will abbreviate the remote home
directory to "/ssh:user@host:~" (for example).
> --- a/lisp/net/tramp-sh.el
> +++ b/lisp/net/tramp-sh.el
> @@ -942,7 +942,8 @@ tramp-vc-registered-read-file-names
> ;; New handlers should be added here.
> ;;;###tramp-autoload
> (defconst tramp-sh-file-name-handler-alist
> - '((access-file . tramp-handle-access-file)
> + '((abbreviate-file-name . tramp-sh-handle-abbreviate-file-name)
> + (access-file . tramp-handle-access-file)
Well, I believe we can implement abbreviation also for other Tramp
backends, like in tramp-sudoedit.el. So it might be better to call this
handler `tramp-handle-abbreviate-file-name'.
> +(defun tramp-sh-handle-abbreviate-file-name (filename)
This shall be in tramp.el then, as `tramp-handle-abbreviate-file-name'.
> + "Like `abbreviate-file-name' for Tramp files."
> + (let* ((case-fold-search (tramp-handle-file-name-case-insensitive-p filename))
Please use `case-insensitive-p'. We don't know whether there will be
other implementation for this magic function in the future. And we shall
not bypass the checks in `tramp-file-name-handler', which are important
for parallel invocations of Tramp handlers.
> + (home-dir
> + (with-parsed-tramp-file-name filename nil
> + (with-tramp-connection-property v "home-directory"
> + (directory-abbrev-apply (tramp-sh-handle-expand-file-name
Same here. Please use `expand-file-name'.
Best regards, Michael.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v3] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-14 14:43 ` Michael Albinus
@ 2021-11-15 6:58 ` Jim Porter
2021-11-15 16:59 ` Michael Albinus
0 siblings, 1 reply; 18+ messages in thread
From: Jim Porter @ 2021-11-15 6:58 UTC (permalink / raw)
To: Michael Albinus; +Cc: 51622
[-- Attachment #1: Type: text/plain, Size: 3219 bytes --]
On 11/14/2021 6:43 AM, Michael Albinus wrote:
> Thanks. However, I believe this test shall be called
> `files-tests-file-name-non-special-file-abbreviate-file-name', like the
> other non-special tests. And perhaps it shall be located prior
> `files-tests-file-name-non-special-access-file'.
Ok, done.
>> 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.
>
> You cannot trust the `tramp-default' property. It is set when a method
> or user or host name is expanded as in "/ssh::". But when the host name
> is used explicitly by the user, as in "/ssh:host:", the property is not
> set, even if "host" is the default. Same for user.
>
> But it shouldn't be too hard to determine the defaults. We have
> tramp-default-method{-alist}, tramp-default-user{-alist}, and
> tramp-default-host{-alist}. All needed information is there.
Right, that confirms what I suspected. I'll try to look into this in
more detail later when I get the chance.
>> 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.)
>
> Good, no regression :-)
Fixing your comments below *did* regress performance for abbreviating
Tramp file names compared to current master (it takes 1.47x as long now
in the worst case), but it's still considerably faster than Emacs 28.
I've attached updated benchmark results to show the difference.
> This shall be rather "---". We don't add documentation (yet) for this
> new Tramp feature.
Fixed.
>> +*** Tramp supports abbreviating remote home directories now.
>> +When calling 'abbreviate-file-name' on a Tramp filename, the result
>> +will abbreviate the home directory to "~".
>
> This might be misleading. ... the result will abbreviate the remote home
> directory to "/ssh:user@host:~" (for example).
Ok, I tried to make this section clearer.
> Well, I believe we can implement abbreviation also for other Tramp
> backends, like in tramp-sudoedit.el. So it might be better to call this
> handler `tramp-handle-abbreviate-file-name'.
Done. I added this for the sudoedit and smb methods, since both support
"~" expansion in `expand-file-name'. That *should* be sufficient, but
I've never used either of those methods, so I could be wrong...
> Please use `case-insensitive-p'. We don't know whether there will be
> other implementation for this magic function in the future. And we shall
> not bypass the checks in `tramp-file-name-handler', which are important
> for parallel invocations of Tramp handlers.
Fixed (same with `expand-file-name'). These changes slow things down a
fair bit, but that's mostly due to checking for the right file name
handler more often. Like I said above though, it's still a lot faster
than Emacs 28 (thanks to bug#51699).
[-- Attachment #2: 0001-Add-another-abbreviate-file-name-test.patch --]
[-- Type: text/plain, Size: 1368 bytes --]
From 4f235215c961f7a2f1c35b1327af7794ab250e4d Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 13 Nov 2021 17:38:36 -0800
Subject: [PATCH 1/2] Add another 'abbreviate-file-name' test
* test/lisp/files-tests.el
(files-tests-file-name-non-special-abbreviate-file-name): New test.
---
test/lisp/files-tests.el | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index d00f1ce326..2c4557ead6 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -465,6 +465,15 @@ files-tests--new-name
(let (file-name-handler-alist)
(concat (file-name-sans-extension name) part (file-name-extension name t))))
+(ert-deftest files-tests-file-name-non-special-abbreviate-file-name ()
+ (let* ((homedir temporary-file-directory)
+ (process-environment (cons (format "HOME=%s" homedir)
+ process-environment))
+ (abbreviated-home-dir nil))
+ ;; Check that abbreviation doesn't occur for quoted file names.
+ (should (equal (concat "/:" homedir "foo/bar")
+ (abbreviate-file-name (concat "/:" homedir "foo/bar"))))))
+
(ert-deftest files-tests-file-name-non-special-access-file ()
(files-tests--with-temp-non-special (tmpfile nospecial)
;; Both versions of the file name work.
--
2.25.1
[-- Attachment #3: 0002-Support-abbreviating-home-directory-of-Tramp-filenam.patch --]
[-- Type: text/plain, Size: 17252 bytes --]
From 8663a872a53f13948096a95ada92e3ac099eee5e Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 14 Nov 2021 22:41:30 -0800
Subject: [PATCH 2/2] Support abbreviating home directory of Tramp filenames
* lisp/files.el (directory-abbrev-make-regexp):
(directory-abbrev-apply): New functions.
(abbreviate-file-name): Check for file name handler.
* lisp/tramp.el (tramp-sh-handle-abbreviate-file-name): New function.
* lisp/files.el (file-name-non-special):
* lisp/net/tramp.el (tramp-file-name-for-operation):
* lisp/net/tramp-sh.el (tramp-sh-file-name-handler-alist):
* lisp/net/tramp-smb.el (tramp-smb-file-name-handler-alist):
* lisp/net/tramp-sudoedit.el (tramp-sudoedit-file-name-handler-alist):
Add 'abbreviate-file-name'.
* test/lisp/net/tramp-tests.el (tramp-test07-abbreviate-file-name):
New test.
* doc/lispref/files.texi (Magic File Names): Mention
'abbreviate-file-name' in the list of magic file name handlers.
* etc/NEWS: Announce the change.
---
doc/lispref/files.texi | 7 +-
etc/NEWS | 11 +++
lisp/files.el | 143 ++++++++++++++++++-----------------
lisp/net/tramp-sh.el | 3 +-
lisp/net/tramp-smb.el | 3 +-
lisp/net/tramp-sudoedit.el | 3 +-
lisp/net/tramp.el | 19 +++++
test/lisp/net/tramp-tests.el | 25 ++++++
8 files changed, 140 insertions(+), 74 deletions(-)
diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index d93770a0d2..4b114ba111 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -3308,8 +3308,8 @@ Magic File Names
@ifnottex
@noindent
-@code{access-file}, @code{add-name-to-file},
-@code{byte-compiler-base-file-name},@*
+@code{abbreviate-file-name}, @code{access-file},
+@code{add-name-to-file}, @code{byte-compiler-base-file-name},@*
@code{copy-directory}, @code{copy-file},
@code{delete-directory}, @code{delete-file},
@code{diff-latest-backup-file},
@@ -3368,7 +3368,8 @@ Magic File Names
@iftex
@noindent
@flushleft
-@code{access-file}, @code{add-name-to-file},
+@code{abbreviate-file-name}, @code{access-file},
+@code{add-name-to-file},
@code{byte-com@discretionary{}{}{}piler-base-file-name},
@code{copy-directory}, @code{copy-file},
@code{delete-directory}, @code{delete-file},
diff --git a/etc/NEWS b/etc/NEWS
index c362e56cee..1738910cbc 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -497,6 +497,14 @@ The newly created buffer will be displayed via 'display-buffer', which
can be customized through the usual mechanism of 'display-buffer-alist'
and friends.
+** Tramp
+
+---
+*** Tramp supports abbreviating remote home directories now.
+When calling 'abbreviate-file-name' on a Tramp filename, the result
+will abbreviate the user's home directory, for example by abbreviating
+"/ssh:user@host:/home/user" to "/ssh:user@host:~".
+
\f
* New Modes and Packages in Emacs 29.1
@@ -632,6 +640,9 @@ This convenience function is useful when writing code that parses
files at run-time, and allows Lisp programs to re-parse files only
when they have changed.
++++
+** 'abbreviate-file-name' now respects magic file name handlers.
+
---
** New function 'font-has-char-p'.
This can be used to check whether a specific font has a glyph for a
diff --git a/lisp/files.el b/lisp/files.el
index 3490d0428a..49bf06bfc1 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -68,6 +68,31 @@ directory-abbrev-alist
:group 'abbrev
:group 'find-file)
+(defun directory-abbrev-make-regexp (directory)
+ "Create a regexp to match DIRECTORY for `directory-abbrev-alist'."
+ (let ((regexp
+ ;; We include a slash at the end, to avoid spurious
+ ;; matches such as `/usr/foobar' when the home dir is
+ ;; `/usr/foo'.
+ (concat "\\`" (regexp-quote directory) "\\(/\\|\\'\\)")))
+ ;; The value of regexp could be multibyte or unibyte. In the
+ ;; latter case, we need to decode it.
+ (if (multibyte-string-p regexp)
+ regexp
+ (decode-coding-string regexp
+ (if (eq system-type 'windows-nt)
+ 'utf-8
+ locale-coding-system)))))
+
+(defun directory-abbrev-apply (filename)
+ "Apply the abbreviations in `directory-abbrev-alist' to FILENAME.
+Note that when calling this, you should set `case-fold-search' as
+appropriate for the filesystem used for FILENAME."
+ (dolist (dir-abbrev directory-abbrev-alist filename)
+ (when (string-match (car dir-abbrev) filename)
+ (setq filename (concat (cdr dir-abbrev)
+ (substring filename (match-end 0)))))))
+
(defcustom make-backup-files t
"Non-nil means make a backup of a file the first time it is saved.
This can be done by renaming the file or by copying.
@@ -2015,73 +2040,54 @@ abbreviate-file-name
started Emacs, set `abbreviated-home-dir' to nil so it will be recalculated)."
;; Get rid of the prefixes added by the automounter.
(save-match-data ;FIXME: Why?
- (if (and automount-dir-prefix
- (string-match automount-dir-prefix filename)
- (file-exists-p (file-name-directory
- (substring filename (1- (match-end 0))))))
- (setq filename (substring filename (1- (match-end 0)))))
- ;; Avoid treating /home/foo as /home/Foo during `~' substitution.
- (let ((case-fold-search (file-name-case-insensitive-p filename)))
- ;; If any elt of directory-abbrev-alist matches this name,
- ;; abbreviate accordingly.
- (dolist (dir-abbrev directory-abbrev-alist)
- (if (string-match (car dir-abbrev) filename)
- (setq filename
- (concat (cdr dir-abbrev)
- (substring filename (match-end 0))))))
- ;; Compute and save the abbreviated homedir name.
- ;; We defer computing this until the first time it's needed, to
- ;; give time for directory-abbrev-alist to be set properly.
- ;; We include a slash at the end, to avoid spurious matches
- ;; such as `/usr/foobar' when the home dir is `/usr/foo'.
- (unless abbreviated-home-dir
- (put 'abbreviated-home-dir 'home (expand-file-name "~"))
- (setq abbreviated-home-dir
- (let* ((abbreviated-home-dir "\\`\\'.") ;Impossible regexp.
- (regexp
- (concat "\\`"
- (regexp-quote
- (abbreviate-file-name
- (get 'abbreviated-home-dir 'home)))
- "\\(/\\|\\'\\)")))
- ;; Depending on whether default-directory does or
- ;; doesn't include non-ASCII characters, the value
- ;; of abbreviated-home-dir could be multibyte or
- ;; unibyte. In the latter case, we need to decode
- ;; it. Note that this function is called for the
- ;; first time (from startup.el) when
- ;; locale-coding-system is already set up.
- (if (multibyte-string-p regexp)
- regexp
- (decode-coding-string regexp
- (if (eq system-type 'windows-nt)
- 'utf-8
- locale-coding-system))))))
-
- ;; If FILENAME starts with the abbreviated homedir,
- ;; and ~ hasn't changed since abbreviated-home-dir was set,
- ;; make it start with `~' instead.
- ;; If ~ has changed, we ignore abbreviated-home-dir rather than
- ;; invalidating it, on the assumption that a change in HOME
- ;; is likely temporary (eg for testing).
- ;; FIXME Is it even worth caching abbreviated-home-dir?
- ;; Ref: https://debbugs.gnu.org/19657#20
- (let (mb1)
- (if (and (string-match abbreviated-home-dir filename)
- (setq mb1 (match-beginning 1))
- ;; If the home dir is just /, don't change it.
- (not (and (= (match-end 0) 1)
- (= (aref filename 0) ?/)))
- ;; MS-DOS root directories can come with a drive letter;
- ;; Novell Netware allows drive letters beyond `Z:'.
- (not (and (memq system-type '(ms-dos windows-nt cygwin))
- (string-match "\\`[a-zA-`]:/\\'" filename)))
- (equal (get 'abbreviated-home-dir 'home)
- (expand-file-name "~")))
- (setq filename
- (concat "~"
- (substring filename mb1))))
- filename))))
+ (if-let ((handler (find-file-name-handler filename 'abbreviate-file-name)))
+ (funcall handler 'abbreviate-file-name filename)
+ (if (and automount-dir-prefix
+ (string-match automount-dir-prefix filename)
+ (file-exists-p (file-name-directory
+ (substring filename (1- (match-end 0))))))
+ (setq filename (substring filename (1- (match-end 0)))))
+ ;; Avoid treating /home/foo as /home/Foo during `~' substitution.
+ (let ((case-fold-search (file-name-case-insensitive-p filename)))
+ ;; If any elt of directory-abbrev-alist matches this name,
+ ;; abbreviate accordingly.
+ (setq filename (directory-abbrev-apply filename))
+
+ ;; Compute and save the abbreviated homedir name.
+ ;; We defer computing this until the first time it's needed, to
+ ;; give time for directory-abbrev-alist to be set properly.
+ (unless abbreviated-home-dir
+ (put 'abbreviated-home-dir 'home (expand-file-name "~"))
+ (setq abbreviated-home-dir
+ (directory-abbrev-make-regexp
+ (let ((abbreviated-home-dir "\\`\\'.")) ;Impossible regexp.
+ (abbreviate-file-name
+ (get 'abbreviated-home-dir 'home))))))
+
+ ;; If FILENAME starts with the abbreviated homedir,
+ ;; and ~ hasn't changed since abbreviated-home-dir was set,
+ ;; make it start with `~' instead.
+ ;; If ~ has changed, we ignore abbreviated-home-dir rather than
+ ;; invalidating it, on the assumption that a change in HOME
+ ;; is likely temporary (eg for testing).
+ ;; FIXME Is it even worth caching abbreviated-home-dir?
+ ;; Ref: https://debbugs.gnu.org/19657#20
+ (let (mb1)
+ (if (and (string-match abbreviated-home-dir filename)
+ (setq mb1 (match-beginning 1))
+ ;; If the home dir is just /, don't change it.
+ (not (and (= (match-end 0) 1)
+ (= (aref filename 0) ?/)))
+ ;; MS-DOS root directories can come with a drive letter;
+ ;; Novell Netware allows drive letters beyond `Z:'.
+ (not (and (memq system-type '(ms-dos windows-nt cygwin))
+ (string-match "\\`[a-zA-`]:/\\'" filename)))
+ (equal (get 'abbreviated-home-dir 'home)
+ (expand-file-name "~")))
+ (setq filename
+ (concat "~"
+ (substring filename mb1))))
+ filename)))))
(defun find-buffer-visiting (filename &optional predicate)
"Return the buffer visiting file FILENAME (a string).
@@ -7836,10 +7842,11 @@ file-name-non-special
;; Get a list of the indices of the args that are file names.
(file-arg-indices
(cdr (or (assq operation
- '(;; The first seven are special because they
+ '(;; The first eight are special because they
;; return a file name. We want to include
;; the /: in the return value. So just
;; avoid stripping it in the first place.
+ (abbreviate-file-name)
(directory-file-name)
(expand-file-name)
(file-name-as-directory)
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index c61025a86b..b83569f3de 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -942,7 +942,8 @@ tramp-vc-registered-read-file-names
;; New handlers should be added here.
;;;###tramp-autoload
(defconst tramp-sh-file-name-handler-alist
- '((access-file . tramp-handle-access-file)
+ '((abbreviate-file-name . tramp-handle-abbreviate-file-name)
+ (access-file . tramp-handle-access-file)
(add-name-to-file . tramp-sh-handle-add-name-to-file)
;; `byte-compiler-base-file-name' performed by default handler.
(copy-directory . tramp-sh-handle-copy-directory)
diff --git a/lisp/net/tramp-smb.el b/lisp/net/tramp-smb.el
index 0b25164902..24119539db 100644
--- a/lisp/net/tramp-smb.el
+++ b/lisp/net/tramp-smb.el
@@ -222,7 +222,8 @@ tramp-smb-actions-set-acl
;; New handlers should be added here.
;;;###tramp-autoload
(defconst tramp-smb-file-name-handler-alist
- '((access-file . tramp-handle-access-file)
+ '((abbreviate-file-name . tramp-handle-abbreviate-file-name)
+ (access-file . tramp-handle-access-file)
(add-name-to-file . tramp-smb-handle-add-name-to-file)
;; `byte-compiler-base-file-name' performed by default handler.
(copy-directory . tramp-smb-handle-copy-directory)
diff --git a/lisp/net/tramp-sudoedit.el b/lisp/net/tramp-sudoedit.el
index 7cf0ea451d..c91bced656 100644
--- a/lisp/net/tramp-sudoedit.el
+++ b/lisp/net/tramp-sudoedit.el
@@ -63,7 +63,8 @@ tramp-sudoedit-sudo-actions
;;;###tramp-autoload
(defconst tramp-sudoedit-file-name-handler-alist
- '((access-file . tramp-handle-access-file)
+ '((abbreviate-file-name . tramp-handle-abbreviate-file-name)
+ (access-file . tramp-handle-access-file)
(add-name-to-file . tramp-sudoedit-handle-add-name-to-file)
(byte-compiler-base-file-name . ignore)
(copy-directory . tramp-handle-copy-directory)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 876bbb2c54..9d4fa485b5 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -2495,6 +2495,8 @@ tramp-file-name-for-operation
file-system-info
;; Emacs 28+ only.
file-locked-p lock-file make-lock-file-name unlock-file
+ ;; Emacs 29+ only.
+ abbreviate-file-name
;; Tramp internal magic file name function.
tramp-set-file-uid-gid))
(if (file-name-absolute-p (nth 0 args))
@@ -3275,6 +3277,23 @@ tramp-handle-file-local-copy-hook
(defvar tramp-handle-write-region-hook nil
"Normal hook to be run at the end of `tramp-*-handle-write-region'.")
+(defun tramp-handle-abbreviate-file-name (filename)
+ "Like `abbreviate-file-name' for Tramp files."
+ (let* ((case-fold-search (file-name-case-insensitive-p filename))
+ (home-dir
+ (with-parsed-tramp-file-name filename nil
+ (with-tramp-connection-property v "home-directory"
+ (directory-abbrev-apply (expand-file-name
+ (tramp-make-tramp-file-name v "~")))))))
+ ;; If any elt of directory-abbrev-alist matches this name,
+ ;; abbreviate accordingly.
+ (setq filename (directory-abbrev-apply filename))
+ (if (string-match (directory-abbrev-make-regexp home-dir) filename)
+ (with-parsed-tramp-file-name filename nil
+ (tramp-make-tramp-file-name
+ v (concat "~" (substring filename (match-beginning 1)))))
+ filename)))
+
(defun tramp-handle-access-file (filename string)
"Like `access-file' for Tramp files."
(setq filename (file-truename filename))
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 52c6159dc1..698d18b528 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -2289,6 +2289,31 @@ tramp-test06-directory-file-name
(should (string-equal (file-name-directory file) file))
(should (string-equal (file-name-nondirectory file) "")))))))
+(ert-deftest tramp-test07-abbreviate-file-name ()
+ "Check that Tramp abbreviates file names correctly."
+ (skip-unless (tramp--test-enabled))
+ (skip-unless (tramp--test-emacs29-p))
+
+ (let* ((remote-host (file-remote-p tramp-test-temporary-file-directory))
+ (home-dir (expand-file-name (concat remote-host "~"))))
+ ;; Check home-dir abbreviation.
+ (should (equal (abbreviate-file-name (concat home-dir "/foo/bar"))
+ (concat remote-host "~/foo/bar")))
+ (should (equal (abbreviate-file-name (concat remote-host
+ "/nowhere/special"))
+ (concat remote-host "/nowhere/special")))
+ ;; Check `directory-abbrev-alist' abbreviation.
+ (let ((directory-abbrev-alist
+ `((,(concat "\\`" (regexp-quote home-dir) "/foo")
+ . ,(concat home-dir "/f"))
+ (,(concat "\\`" (regexp-quote remote-host) "/nowhere")
+ . ,(concat remote-host "/nw")))))
+ (should (equal (abbreviate-file-name (concat home-dir "/foo/bar"))
+ (concat remote-host "~/f/bar")))
+ (should (equal (abbreviate-file-name (concat remote-host
+ "/nowhere/special"))
+ (concat remote-host "/nw/special"))))))
+
(ert-deftest tramp-test07-file-exists-p ()
"Check `file-exist-p', `write-region' and `delete-file'."
(skip-unless (tramp--test-enabled))
--
2.25.1
[-- Attachment #4: benchmark-results.txt --]
[-- Type: text/plain, Size: 2555 bytes --]
Emacs 29 master
---------------
Empty ‘directory-abbrev-alist’
Local | Elapsed time: 0.082094s (0.012981s in 1 GCs)
Tramp | Elapsed time: 0.570441s (0.175013s in 13 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.334320s (0.129351s in 10 GCs)
Tramp | Elapsed time: 0.838366s (0.294330s in 22 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.530187s (0.320165s in 25 GCs)
Tramp | Elapsed time: 0.836190s (0.285371s in 22 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.089229s (0.492225s in 38 GCs)
Tramp | Elapsed time: 1.587351s (0.649014s in 50 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.292103s (0.687631s in 53 GCs)
Tramp | Elapsed time: 1.592210s (0.651090s in 50 GCs)
With old patch (from Nov 13)
----------------------------
Empty ‘directory-abbrev-alist’
Local | Elapsed time: 0.076136s (0.012949s in 1 GCs)
Tramp | Elapsed time: 0.510445s (0.160052s in 12 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.342509s (0.130916s in 10 GCs)
Tramp | Elapsed time: 0.780201s (0.281118s in 21 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.538353s (0.323898s in 25 GCs)
Tramp | Elapsed time: 0.699262s (0.245942s in 19 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.104786s (0.500880s in 38 GCs)
Tramp | Elapsed time: 1.524662s (0.642236s in 49 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.299160s (0.687867s in 53 GCs)
Tramp | Elapsed time: 1.519829s (0.651691s in 47 GCs)
With new patch (from Nov 14)
----------------------------
Empty ‘directory-abbrev-alist’
Local | Elapsed time: 0.077384s (0.013098s in 1 GCs)
Tramp | Elapsed time: 0.839370s (0.267310s in 20 GCs)
100 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 0.338255s (0.131483s in 10 GCs)
Tramp | Elapsed time: 1.084413s (0.370287s in 28 GCs)
100 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 0.546285s (0.333527s in 25 GCs)
Tramp | Elapsed time: 1.038340s (0.359871s in 27 GCs)
500 items in ‘directory-abbrev-alist’ (no matches)
Local | Elapsed time: 1.099473s (0.500838s in 38 GCs)
Tramp | Elapsed time: 1.848707s (0.747623s in 56 GCs)
500 items in ‘directory-abbrev-alist’ (all matches)
Local | Elapsed time: 1.313968s (0.704238s in 53 GCs)
Tramp | Elapsed time: 1.802593s (0.727102s in 55 GCs)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v3] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-15 6:58 ` bug#51622: 29.0.50; [PATCH v3] " Jim Porter
@ 2021-11-15 16:59 ` Michael Albinus
2021-11-16 1:14 ` Jim Porter
2021-11-16 12:57 ` Michael Albinus
0 siblings, 2 replies; 18+ messages in thread
From: Michael Albinus @ 2021-11-15 16:59 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622
Jim Porter <jporterbugs@gmail.com> writes:
Hi Jim,
>> Thanks. However, I believe this test shall be called
>> `files-tests-file-name-non-special-file-abbreviate-file-name', like the
>> other non-special tests. And perhaps it shall be located prior
>> `files-tests-file-name-non-special-access-file'.
>
> Ok, done.
Thanks, I've pushed this to the master branch.
>> But it shouldn't be too hard to determine the defaults. We have
>> tramp-default-method{-alist}, tramp-default-user{-alist}, and
>> tramp-default-host{-alist}. All needed information is there.
>
> Right, that confirms what I suspected. I'll try to look into this in
> more detail later when I get the chance.
Would be interesting to see the result :-)
>>> 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.)
>> Good, no regression :-)
>
> Fixing your comments below *did* regress performance for abbreviating
> Tramp file names compared to current master (it takes 1.47x as long
> now in the worst case), but it's still considerably faster than Emacs
> 28. I've attached updated benchmark results to show the difference.
Yes, that's the price we have to pay for clean code. I've explained why
it is needed.
However, the `tramp-file-name-handler' machinery has been grown over 20+
years. I'm not aware of useless stuff, but maybe it's worth to have an
*external* look at this wrt performance. Are you interested?
>> Well, I believe we can implement abbreviation also for other Tramp
>> backends, like in tramp-sudoedit.el. So it might be better to call this
>> handler `tramp-handle-abbreviate-file-name'.
>
> Done. I added this for the sudoedit and smb methods, since both support
> "~" expansion in `expand-file-name'. That *should* be sufficient, but
> I've never used either of those methods, so I could be wrong...
Thanks. I didn't find the time to check it for these methods, but it's
on my TODO for next days.
>> Please use `case-insensitive-p'. We don't know whether there will be
>> other implementation for this magic function in the future. And we shall
>> not bypass the checks in `tramp-file-name-handler', which are important
>> for parallel invocations of Tramp handlers.
>
> Fixed (same with `expand-file-name'). These changes slow things down a
> fair bit, but that's mostly due to checking for the right file name
> handler more often. Like I said above though, it's still a lot faster
> than Emacs 28 (thanks to bug#51699).
I've committed everything to master. Then I ran the regression tests,
and there were indeed some few surprises. All of them shall be fixed now
with my commit after yours.
I tend to close this bug report now, since everything reported has been
implemented. The open points don't need this bug anymore for progress.
WDYT?
Best regards, Michael.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v3] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-15 16:59 ` Michael Albinus
@ 2021-11-16 1:14 ` Jim Porter
2021-11-16 11:43 ` Michael Albinus
2021-11-16 12:57 ` Michael Albinus
1 sibling, 1 reply; 18+ messages in thread
From: Jim Porter @ 2021-11-16 1:14 UTC (permalink / raw)
To: Michael Albinus; +Cc: 51622
On 11/15/2021 8:59 AM, Michael Albinus wrote:
>> Fixing your comments below *did* regress performance for abbreviating
>> Tramp file names compared to current master (it takes 1.47x as long
>> now in the worst case), but it's still considerably faster than Emacs
>> 28. I've attached updated benchmark results to show the difference.
>
> Yes, that's the price we have to pay for clean code. I've explained why
> it is needed.
Agreed. It's better to be "slow and right" than "fast and wrong". :)
> I've committed everything to master. Then I ran the regression tests,
> and there were indeed some few surprises. All of them shall be fixed now
> with my commit after yours.
Thanks for merging everything. I'll be sure to keep the changes in your
followup commit in mind (especially the `tramp-compat-funcall' parts) if
I make any future Tramp patches.
> I tend to close this bug report now, since everything reported has been
> implemented. The open points don't need this bug anymore for progress.
>
> WDYT?
Sounds good to me, we can close this.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v3] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-16 1:14 ` Jim Porter
@ 2021-11-16 11:43 ` Michael Albinus
0 siblings, 0 replies; 18+ messages in thread
From: Michael Albinus @ 2021-11-16 11:43 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622-done
Version: 29.1
Jim Porter <jporterbugs@gmail.com> writes:
Hi Jim,
>> I tend to close this bug report now, since everything reported has been
>> implemented. The open points don't need this bug anymore for progress.
>> WDYT?
>
> Sounds good to me, we can close this.
Thanks for the feedback, closing the bug.
Best regards, Michael.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#51622: 29.0.50; [PATCH v3] Abbreviate remote home directories in `abbreviate-file-name'
2021-11-15 16:59 ` Michael Albinus
2021-11-16 1:14 ` Jim Porter
@ 2021-11-16 12:57 ` Michael Albinus
1 sibling, 0 replies; 18+ messages in thread
From: Michael Albinus @ 2021-11-16 12:57 UTC (permalink / raw)
To: Jim Porter; +Cc: 51622
Michael Albinus <michael.albinus@gmx.de> writes:
Hi Jim,
>> Done. I added this for the sudoedit and smb methods, since both support
>> "~" expansion in `expand-file-name'. That *should* be sufficient, but
>> I've never used either of those methods, so I could be wrong...
>
> Thanks. I didn't find the time to check it for these methods, but it's
> on my TODO for next days.
FTR, I've just tested both methods, and they work out-of-the-box, as expected.
Best regards, Michael.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-11-16 12:57 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-06 3:44 bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name' Jim Porter
2021-11-06 8:06 ` Eli Zaretskii
2021-11-06 15:34 ` Michael Albinus
2021-11-06 16:38 ` Jim Porter
2021-11-06 17:41 ` Michael Albinus
2021-11-07 3:30 ` bug#51622: 29.0.50; [PATCH v2] " Jim Porter
2021-11-07 18:37 ` Michael Albinus
2021-11-08 4:54 ` Jim Porter
2021-11-08 15:58 ` Michael Albinus
2021-11-08 18:32 ` Jim Porter
2021-11-08 19:18 ` Michael Albinus
2021-11-14 2:10 ` Jim Porter
2021-11-14 14:43 ` Michael Albinus
2021-11-15 6:58 ` bug#51622: 29.0.50; [PATCH v3] " Jim Porter
2021-11-15 16:59 ` Michael Albinus
2021-11-16 1:14 ` Jim Porter
2021-11-16 11:43 ` Michael Albinus
2021-11-16 12:57 ` Michael Albinus
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).