GNU bug report logs - #34221
[PATCH] Make project-files work with remote files

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 34221 in the body.
You can then email your comments to 34221 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Felicián Németh <felician.nemeth <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Make project-files work with remote files
Date: Sun, 27 Jan 2019 13:21:04 +0100

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

Hi,

I'd like project-files to work even if the project is located on a
remote system.  The patch attached is my attempt to achieve that.
This is my first contribution to Emacs, so any feedback is welcomed.

Thanks,
Felicián

[0001-Make-project-files-work-with-remote-files.patch (text/x-patch, attachment)]

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Felicián Németh <felician.nemeth <at> gmail.com>
Cc: 34221 <at> debbugs.gnu.org
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Mon, 28 Jan 2019 08:12:34 +0100

Felicián Németh <felician.nemeth <at> gmail.com> writes:

> Hi,

Hi Felicián,

> +(defun project--command-to-string (command)
> +  "Execute shell command COMMAND and return its output as a string.
> +Similar to `shell-command-to-string', but calls
> +`process-file-shell-command'."
> +  (with-temp-buffer
> +    (process-file-shell-command command nil t)
> +    (buffer-string)))

Why that? `shell-command-to-string' works also on remote hosts.

> Thanks,
> Felicián

Best regards, Michael.

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

From: Felicián Németh <felician.nemeth <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 34221 <at> debbugs.gnu.org
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Mon, 28 Jan 2019 09:10:25 +0100

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

> Why that? `shell-command-to-string' works also on remote hosts.

That wasn't clear from the documentation and I haven't tried it out :(
Anyway, I've attached a new, simplified version.
However, now I wonder whether project--remote-file-name would be more
useful if it would derive the remote-id from default-directory.  Or
should I simply eliminate this helper function altogether?

Thanks,
Felicián

[0001-Make-project-files-work-with-remote-files.patch (text/x-patch, attachment)]

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Felicián Németh <felician.nemeth <at> gmail.com>
Cc: 34221 <at> debbugs.gnu.org
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Mon, 28 Jan 2019 09:29:12 +0100

Felicián Németh <felician.nemeth <at> gmail.com> writes:

Hi Felicián,

> However, now I wonder whether project--remote-file-name would be more
> useful if it would derive the remote-id from default-directory.

Yes, that's the usual approach.

> should I simply eliminate this helper function altogether?

Dunno, looks now like just a convenience function.

> Thanks,
> Felicián

Best regards, Michael.

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

From: Felicián Németh <felician.nemeth <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 34221 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Wed, 30 Jan 2019 09:15:22 +0100

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

Hi Michael,

I've attached another version of the patch.  Hopefully, this will be
the last one.

Thanks again,
Felicián

[0001-Make-project-files-work-with-remote-files.patch (text/x-patch, attachment)]

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Felicián Németh <felician.nemeth <at> gmail.com>
Cc: 34221 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Wed, 30 Jan 2019 09:34:41 +0100

Felicián Németh <felician.nemeth <at> gmail.com> writes:

> Hi Michael,

Hi Felicián,

> I've attached another version of the patch.  Hopefully, this will be
> the last one.

LGTM. Let Dmitry comment, and if he agrees, it could be pushed.

> Thanks again,
> Felicián

Best regards, Michael.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Michael Albinus <michael.albinus <at> gmx.de>,
 Felicián Németh <felician.nemeth <at> gmail.com>
Cc: 34221 <at> debbugs.gnu.org
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Fri, 1 Feb 2019 06:16:54 +0300

On 30.01.2019 11:34, Michael Albinus wrote:
> LGTM. Let Dmitry comment, and if he agrees, it could be pushed.

Looks good to me, except project--file-remote-name could do with a 
rename. At least call it ...-remote-nameS, maybe?

Or project--expand-remote-names.

Aside from that, '(concat remote-id file)' looks a bit iffy to me, but 
since you approved it, it must be fine.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 34221 <at> debbugs.gnu.org,
 Felicián Németh <felician.nemeth <at> gmail.com>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Fri, 01 Feb 2019 09:14:45 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

Hi Dmitry,

> Aside from that, '(concat remote-id file)' looks a bit iffy to me, but
> since you approved it, it must be fine.

It's according to the docs. remote-id is the result of a file-remote-p
call, which says in its docstring

--8<---------------cut here---------------start------------->8---
Tip: You can use this expansion of remote identifier components
     to derive a new remote file name from an existing one.  For
     example, if FILE is "/sudo::/path/to/file" then

       (concat (file-remote-p FILE) "/bin/sh")

     returns a remote file name for file "/bin/sh" that has the
     same remote identifier as FILE but expanded; a name such as
     "/sudo:root <at> myhost:/bin/sh".
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 34221 <at> debbugs.gnu.org,
 Felicián Németh <felician.nemeth <at> gmail.com>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Sat, 2 Feb 2019 03:10:49 +0300

On 01.02.2019 11:14, Michael Albinus wrote:
> It's according to the docs. remote-id is the result of a file-remote-p
> call, which says in its docstring

I see.

If you like to push the patch yourself, please go ahead.

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

From: Felicián Németh <felician.nemeth <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 34221 <at> debbugs.gnu.org, Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Sat, 2 Feb 2019 09:36:48 +0100

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

Hello Dmitry,

Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>
> Looks good to me, except project--file-remote-name could do with a
> rename. At least call it ...-remote-nameS, maybe?

I don't know if it is still necessary, but I updated the patch with
renaming the defun in question by appending an "s" to its name.

Thanks,
Felicián
(ps. I've singed the copyright papers.)

[0001-Make-project-files-work-with-remote-files.patch (text/x-patch, attachment)]

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Felicián Németh <felician.nemeth <at> gmail.com>
Cc: 34221 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Sat, 02 Feb 2019 10:18:11 +0100

Felicián Németh <felician.nemeth <at> gmail.com> writes:

> Hello Dmitry,

Hi Felicián,

> I don't know if it is still necessary, but I updated the patch with
> renaming the defun in question by appending an "s" to its name.

I'm ready to commit it in your name. Reading the patch again, it might
be possible to simplify the code further.

> +(defun project--file-remote-names (local-files)
> +  "Return LOCAL-FILES as if they were on the system of `default-directory'."
> +  (let ((remote-id (file-remote-p default-directory)))
> +    (if (not remote-id)
> +        local-files
> +      (mapcar (lambda (file)
> +                (concat remote-id file))
> +              local-files))))

concat accepts nil as argument. So the function could be rewritten:

(defun project--file-remote-names (local-files)
  "Return LOCAL-FILES as if they were on the system of `default-directory'."
  (let ((remote-id (file-remote-p default-directory)))
    (mapcar (lambda (file)
              (concat remote-id file))
            local-files))))

With this simple body, it might even not needed as function. We would
have then (untested)

  (let ((default-directory dir)
        (remote-id (file-remote-p dir)

...

    (mapcar (lambda (file) (concat remote-id file))
            (split-string (shell-command-to-string command) "\0" t))

WDYT? Do you want to prepare a patch along these lines?

> Thanks,
> Felicián

Best regards, Michael.

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

From: Felicián Németh <felician.nemeth <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 34221 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Sat, 2 Feb 2019 11:16:25 +0100

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

Hi Michael,

[...]
> With this simple body, it might even not needed as function. We would
> have then (untested)
>
>   (let ((default-directory dir)
>         (remote-id (file-remote-p dir)
>
> ...
>
>     (mapcar (lambda (file) (concat remote-id file))
>             (split-string (shell-command-to-string command) "\0" t))
>
> WDYT? Do you want to prepare a patch along these lines?

Originally, I wanted to avoid this approach, because mapcar iterates
over the result even if remote-id is nil, but I guess the overhead is
negligible. I've attached the simplified patch.

Thanks,
Felicián

[0001-Make-project-files-work-with-remote-files.patch (text/x-patch, attachment)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Felicián Németh <felician.nemeth <at> gmail.com>,
 Michael Albinus <michael.albinus <at> gmx.de>
Cc: 34221 <at> debbugs.gnu.org
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Sat, 2 Feb 2019 14:59:18 +0300

On 02.02.2019 13:16, Felicián Németh wrote:
> Originally, I wanted to avoid this approach, because mapcar iterates
> over the result even if remote-id is nil, but I guess the overhead is
> negligible.

That's how I read it too. I'm not so sure that the overhead is negligible.

Compare

(benchmark 1 '(all-completions "" obarray))

and

(benchmark 1 '(mapcar (lambda (s) (concat nil s)) (all-completions "" 
obarray)))

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 34221 <at> debbugs.gnu.org,
 Felicián Németh <felician.nemeth <at> gmail.com>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Sat, 02 Feb 2019 13:13:51 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> That's how I read it too. I'm not so sure that the overhead is negligible.
>
> Compare
>
> (benchmark 1 '(all-completions "" obarray))
>
> and
>
> (benchmark 1 '(mapcar (lambda (s) (concat nil s)) (all-completions ""
> obarray)))

Sure, I'm aware of this difference. But there is (length obarray) => 15121
in my running Emacs stanza, and I doubt that so many files are handled
in project-files at once. And even if there are so many files, the other
operations on them will rule out this difference. So yes, I believe
this is negligible.

Best regards, Michael.

Message #49 received at 34221-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Felicián Németh <felician.nemeth <at> gmail.com>
Cc: 34221-done <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Sat, 02 Feb 2019 13:26:51 +0100

Felicián Németh <felician.nemeth <at> gmail.com> writes:

> Hi Michael,

Hi Felicián,

> Originally, I wanted to avoid this approach, because mapcar iterates
> over the result even if remote-id is nil, but I guess the overhead is
> negligible. I've attached the simplified patch.

Thanks, I've pushed it to the master branch. Closing the bug.

> Thanks,
> Felicián

Best regards, Michael.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 34221 <at> debbugs.gnu.org,
 Felicián Németh <felician.nemeth <at> gmail.com>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Thu, 7 Feb 2019 14:05:54 +0300

On 02.02.2019 15:13, Michael Albinus wrote:

> Sure, I'm aware of this difference. But there is (length obarray) => 15121
> in my running Emacs stanza, and I doubt that so many files are handled
> in project-files at once.

Emacs contains ~4000 files.

> And even if there are so many files, the other
> operations on them will rule out this difference. So yes, I believe
> this is negligible.

Why not benchmark, then? I see a stable difference in project-files's 
runtime, on the order of 1-4%.

It's not a lot, but why create conses when we don't have to.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 34221 <at> debbugs.gnu.org,
 Felicián Németh <felician.nemeth <at> gmail.com>
Subject: Re: bug#34221: [PATCH] Make project-files work with remote files
Date: Thu, 07 Feb 2019 12:14:26 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

Hi Dmitry,

>> And even if there are so many files, the other
>> operations on them will rule out this difference. So yes, I believe
>> this is negligible.
>
> Why not benchmark, then? I see a stable difference in project-files's
> runtime, on the order of 1-4%.
>
> It's not a lot, but why create conses when we don't have to.

No problem from my side, change it as you believe it is appropriate.

Best regards, Michael.

Previous Next

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