[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `ab
From: |
Michael Albinus |
Subject: |
bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name' |
Date: |
Sun, 07 Nov 2021 19:37:55 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) |
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.
- bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name', Jim Porter, 2021/11/05
- bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name', Eli Zaretskii, 2021/11/06
- bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name', Michael Albinus, 2021/11/06
- bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name', Jim Porter, 2021/11/06
- bug#51622: 29.0.50; [PATCH] Abbreviate remote home directories in `abbreviate-file-name', Michael Albinus, 2021/11/06
- bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name', Jim Porter, 2021/11/06
- bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name',
Michael Albinus <=
- bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name', Jim Porter, 2021/11/07
- bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name', Michael Albinus, 2021/11/08
- bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name', Jim Porter, 2021/11/08
- bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name', Michael Albinus, 2021/11/08
- bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name', Jim Porter, 2021/11/13
- bug#51622: 29.0.50; [PATCH v2] Abbreviate remote home directories in `abbreviate-file-name', Michael Albinus, 2021/11/14
- bug#51622: 29.0.50; [PATCH v3] Abbreviate remote home directories in `abbreviate-file-name', Jim Porter, 2021/11/15
- bug#51622: 29.0.50; [PATCH v3] Abbreviate remote home directories in `abbreviate-file-name', Michael Albinus, 2021/11/15
- bug#51622: 29.0.50; [PATCH v3] Abbreviate remote home directories in `abbreviate-file-name', Jim Porter, 2021/11/15
- bug#51622: 29.0.50; [PATCH v3] Abbreviate remote home directories in `abbreviate-file-name', Michael Albinus, 2021/11/16