From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Michael Albinus Newsgroups: gmane.emacs.bugs 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 Message-ID: <87sfw7u7zw.fsf@gmx.de> References: <5ac0b5f3-302c-2f96-771c-8d38088aa573@gmail.com> <87mtmhmh60.fsf@gmx.de> <87a6ihchbo.fsf@gmx.de> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40037"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: 51622@debbugs.gnu.org To: Jim Porter Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Nov 07 19:39:51 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mjn54-000AG6-6G for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 07 Nov 2021 19:39:50 +0100 Original-Received: from [::1] (port=53990 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mjn52-0001YR-Ju for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 07 Nov 2021 13:39:48 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:39558) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mjn4O-0001Xd-SP for bug-gnu-emacs@gnu.org; Sun, 07 Nov 2021 13:39:09 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:43162) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mjn4I-0005yK-DV for bug-gnu-emacs@gnu.org; Sun, 07 Nov 2021 13:39:07 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mjn4I-0007z2-6y for bug-gnu-emacs@gnu.org; Sun, 07 Nov 2021 13:39:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Michael Albinus Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 07 Nov 2021 18:39:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 51622 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 51622-submit@debbugs.gnu.org id=B51622.163631028930620 (code B ref 51622); Sun, 07 Nov 2021 18:39:02 +0000 Original-Received: (at 51622) by debbugs.gnu.org; 7 Nov 2021 18:38:09 +0000 Original-Received: from localhost ([127.0.0.1]:54708 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mjn3Q-0007xn-JU for submit@debbugs.gnu.org; Sun, 07 Nov 2021 13:38:09 -0500 Original-Received: from mout.gmx.net ([212.227.17.22]:41377) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mjn3L-0007xE-Nv for 51622@debbugs.gnu.org; Sun, 07 Nov 2021 13:38:07 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1636310277; bh=0BdenhQfk9tvpiGFxPzjRojCGv/WsO2R1x2G4LRANHA=; h=X-UI-Sender-Class:From:To:Cc:Subject:References:Date:In-Reply-To; b=eZd6JTiNZ23aolgIpOeXQyUpXzZj0IHdkBjP3DopNbRJc3a4hoNiO08jLwb955iGs 1IBqeutvyDIK8qdOfnGOtvVdPiGsOgqbHDq/m+cz9M1mrokFYTiA2B4mXO0DNC319i OGJ9ZLLrci6jSBAn/J7lHWmzPxD/L9Lmpvl9e12U= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Original-Received: from gandalf.gmx.de ([213.220.148.37]) by mail.gmx.net (mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MnakX-1mKvxp38ZZ-00jczy; Sun, 07 Nov 2021 19:37:56 +0100 In-Reply-To: (Jim Porter's message of "Sat, 6 Nov 2021 20:30:39 -0700") X-Provags-ID: V03:K1:NrMd8vJYhwkGvw2p4TCDuqAIG7jp+oJ3ts+N9Tky/gKBf4UEhH9 7n3m/x4N3zDVZCZgPU8Tos25tJOd+Fz0FiZVeZM7cHDCd05VRQ+cCVwqDh8LLDxUKxAb4kJ 41bPt/p3KIbMTc6vGYrRI68BNw6kckAWoDwyBFpNX+3GjpsSCebwT1i3j4e0R7Y2VlH9jZw TNasKztyqEDGvnGhPfsdQ== X-UI-Out-Filterresults: notjunk:1;V03:K0:iuN9SXVM4lY=:aTjjf+xYaqaqR2XLPbq28w bo/sfBNSCuerADc/STBvGnRIL+L2mKbk0Ck8MOal4d8bokTRL43Kwfh7zxBN59QDWVr3aLILK EHStvwMHHOG+5gJuaZYBkTRK29ThMX4eQiW7PcOs2u0bfEIQ1NPy6CRHb2HnW+ZYnljMIi1Z3 CuC5HIR99D6fZszGh7bRsuEhvtpBdmbKx5+ax67QgFkdBr2ewH+feB4awxBNBdhUKkTRBS85N rNWeLx0yRM7WehHdccUCnPP+xc39/spdVbXnoxlc40FNZ/DAeU6//hAqxqZ1S+58stSkNfafE vLUrMuxPXI2wf0l4oB+waHJNSnfLI7UrihOdDKLFwy7QwXPyOgimhsqiHn85wy2uiYFoXIrW/ 516I+Ue09INOH0E1AtkejqLFxl23BZgdk4u2tOlcqqTvu3jEFsNSjVxKerPBpYm2Y5Bvd4+ZP p01tmDj10egTaBPqqB6SOJQXaQPnCJqNbabEsD9bbioJva9NXMx7DnVxE6oAuSOnXs5JPYQBb n3lHwwDGQGFXMwsXKHLUE6R5HIdzyeYbM5iQGAq2jrCZJUx9ICvgZ6ozSwwkRoTosLTUfmpn+ TUH50q/gEgA3VwkRYZ6DlSqJBpa+WqFZZ1igZ5l7A87pJyOnj3MjW5iRHzFurXv8Z51ZQzOS2 fSF5pTFi+TGnhiMI2Hn9HMVK/Rc4TWijqaG9elQAtY+aJePkvnAy58qR52nzidx+E7Rjyyozn 4AS7HeGfaWhZw8hWnz7pmasRbqfcFOpcbyP2vGeGR3OPSSwzng2flwWmdYoeM/RCUk4FNdKw X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:219279 Archived-At: Jim Porter 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.