GNU bug report logs - #64531
[PATCH] Add new command vc-dir-copy-filename-as-kill

Previous Next

To reply to this bug, email your comments to 64531 AT debbugs.gnu.org.

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

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

From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add new command vc-dir-copy-filename-as-kill
Date: Sat, 08 Jul 2023 10:51:53 +0300

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

Hi!

This new command is inspired by `dired-copy-filename-as-kill', but has a
few differences, that can be easily reversed if there is such need.

Changes:
1. even single filename is quoted;
2. filenames are relative by default, prefix makes them absolute;
3. value of prefix argument does not matter, in dired it has to be 0.

I bound it to the `w' key because `w' is currently unbound in
vc-dir-mode-map and it is what is used in dired.

[0001-Add-new-command-vc-dir-copy-filename-as-kill.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
Cc: 64531 <at> debbugs.gnu.org
Subject: Re: bug#64531: [PATCH] Add new command vc-dir-copy-filename-as-kill
Date: Sat, 08 Jul 2023 12:25:13 +0300

> From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
> Date: Sat, 08 Jul 2023 10:51:53 +0300
> 
> This new command is inspired by `dired-copy-filename-as-kill', but has a
> few differences, that can be easily reversed if there is such need.
> 
> Changes:
> 1. even single filename is quoted;
> 2. filenames are relative by default, prefix makes them absolute;
> 3. value of prefix argument does not matter, in dired it has to be 0.
> 
> I bound it to the `w' key because `w' is currently unbound in
> vc-dir-mode-map and it is what is used in dired.

Thanks.  However, new commands should be called out in NEWS, and
commands that are important enough should also be mentioned in the
user manual.

> +(defun vc-dir-copy-filename-as-kill (&optional absolutep)
> +  "Copy names of marked files (or file under cursor) into the kill ring.

This sentence is too general: it doesn't mention VC Dir, although
AFAIU it is specific to vc-dir.

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

From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64531 <at> debbugs.gnu.org
Subject: bug#64531: [PATCH v2] Add new command vc-dir-copy-filename-as-kill
Date: Tue, 08 Aug 2023 16:44:56 +0300

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

Eli Zaretskii <eliz <at> gnu.org> writes:

> However, new commands should be called out in NEWS, and commands that
> are important enough should also be mentioned in the user manual.

Added entry in NEWS, but not in the user manual, there is no such for
vc-dir afaik.

>> +(defun vc-dir-copy-filename-as-kill (&optional absolutep)
>> +  "Copy names of marked files (or file under cursor) into the kill ring.
>
> This sentence is too general: it doesn't mention VC Dir, although
> AFAIU it is specific to vc-dir.

Fixed.

[v2-0001-Add-new-command-vc-dir-copy-filename-as-kill.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
Cc: 64531 <at> debbugs.gnu.org
Subject: Re: bug#64531: [PATCH v2] Add new command vc-dir-copy-filename-as-kill
Date: Tue, 08 Aug 2023 16:56:26 +0300

> From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
> Cc: 64531 <at> debbugs.gnu.org
> Date: Tue, 08 Aug 2023 16:44:56 +0300
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > However, new commands should be called out in NEWS, and commands that
> > are important enough should also be mentioned in the user manual.
> 
> Added entry in NEWS, but not in the user manual, there is no such for
> vc-dir afaik.

vc-dir is described in the subsection "VC Directory Mode" and its
sub-subsections.

> +(defun vc-dir-copy-filename-as-kill (&optional absolutep)
> +  "Copy filenames from the `vc-dir' buffer into the kill ring.
> +If there are marked files, copy their names, otherwise copy the
> +file at point.  Names will be separated by a space, each will be
> +quoted using `shell-quote-argument'.  If ABSOLUTEP use absolute
                                                     ^
A comma is missing there.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>, Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 64531 <at> debbugs.gnu.org
Subject: Re: bug#64531: [PATCH v2] Add new command vc-dir-copy-filename-as-kill
Date: Sat, 19 Aug 2023 11:06:56 +0300

> From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
> Cc: 64531 <at> debbugs.gnu.org
> Date: Tue, 08 Aug 2023 16:44:56 +0300
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > However, new commands should be called out in NEWS, and commands that
> > are important enough should also be mentioned in the user manual.
> 
> Added entry in NEWS, but not in the user manual, there is no such for
> vc-dir afaik.
> 
> >> +(defun vc-dir-copy-filename-as-kill (&optional absolutep)
> >> +  "Copy names of marked files (or file under cursor) into the kill ring.
> >
> > This sentence is too general: it doesn't mention VC Dir, although
> > AFAIU it is specific to vc-dir.
> 
> Fixed.

Dmitry, is this good to go in?

I still think it should be mention in the Emacs user manual, where we
document vc-dir.

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

From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64531 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#64531: [PATCH v2] Add new command vc-dir-copy-filename-as-kill
Date: Sun, 20 Aug 2023 02:12:32 +0300

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
>> Cc: 64531 <at> debbugs.gnu.org
>> Date: Tue, 08 Aug 2023 16:44:56 +0300
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > However, new commands should be called out in NEWS, and commands that
>> > are important enough should also be mentioned in the user manual.
>> 
>> Added entry in NEWS, but not in the user manual, there is no such for
>> vc-dir afaik.
>> 
>> >> +(defun vc-dir-copy-filename-as-kill (&optional absolutep)
>> >> +  "Copy names of marked files (or file under cursor) into the kill ring.
>> >
>> > This sentence is too general: it doesn't mention VC Dir, although
>> > AFAIU it is specific to vc-dir.
>> 
>> Fixed.
>
> Dmitry, is this good to go in?
>
> I still think it should be mention in the Emacs user manual, where we
> document vc-dir.

Hi, Eli!

I am working on the 3rd version of the patch with all the necessary
documentation and changes to the command which I will explain in the
cover letter.  Not sure when I'll finish it though.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
Cc: 64531 <at> debbugs.gnu.org
Subject: Re: bug#64531: [PATCH v2] Add new command vc-dir-copy-filename-as-kill
Date: Sun, 20 Aug 2023 02:33:59 +0300

Hi Eli,

On 19/08/2023 11:06, Eli Zaretskii wrote:
> Dmitry, is this good to go in?

I'm good with the binding and the general functionality.

The difference from dired-copy-filename-as-kill in handing of the prefix 
argument looks unfortunate (as a principle), but I'd rather leave it for 
you to judge. Perhaps changing the latter command's interface would be a 
better choice; I'm not sure why it works that way.

Implementation difference: instead of going through (format "%S"), this 
one uses shell-quote-argument. Is that good enough for Windows systems?

And one more difference which we might want to see fixed:

  (When there's a single file, no quoting is done.)

It seems like, at least, in some usage scenarios users would prefer to 
have copied filename not quoted. E.g. for subsequent use with find-file 
or... something similar. I don't really use this command all that much, 
so maybe I'm mistaken here, though.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
Cc: 64531 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: bug#64531: [PATCH v2] Add new command vc-dir-copy-filename-as-kill
Date: Sun, 20 Aug 2023 09:20:01 +0300

> From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
> Cc: Dmitry Gutov <dgutov <at> yandex.ru>,  64531 <at> debbugs.gnu.org
> Date: Sun, 20 Aug 2023 02:12:32 +0300
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
> >> Cc: 64531 <at> debbugs.gnu.org
> >> Date: Tue, 08 Aug 2023 16:44:56 +0300
> >> 
> >> Eli Zaretskii <eliz <at> gnu.org> writes:
> >> 
> >> > However, new commands should be called out in NEWS, and commands that
> >> > are important enough should also be mentioned in the user manual.
> >> 
> >> Added entry in NEWS, but not in the user manual, there is no such for
> >> vc-dir afaik.
> >> 
> >> >> +(defun vc-dir-copy-filename-as-kill (&optional absolutep)
> >> >> +  "Copy names of marked files (or file under cursor) into the kill ring.
> >> >
> >> > This sentence is too general: it doesn't mention VC Dir, although
> >> > AFAIU it is specific to vc-dir.
> >> 
> >> Fixed.
> >
> > Dmitry, is this good to go in?
> >
> > I still think it should be mention in the Emacs user manual, where we
> > document vc-dir.
> 
> Hi, Eli!
> 
> I am working on the 3rd version of the patch with all the necessary
> documentation and changes to the command which I will explain in the
> cover letter.  Not sure when I'll finish it though.

Good to hear, and TIA.

There's no rush, so take your time.  (I will ping again if it takes
too long IMO ;-)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 64531 <at> debbugs.gnu.org, ivan-p-sokolov <at> ya.ru
Subject: Re: bug#64531: [PATCH v2] Add new command vc-dir-copy-filename-as-kill
Date: Sun, 20 Aug 2023 09:26:12 +0300

> Date: Sun, 20 Aug 2023 02:33:59 +0300
> Cc: 64531 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> 
> The difference from dired-copy-filename-as-kill in handing of the prefix 
> argument looks unfortunate (as a principle), but I'd rather leave it for 
> you to judge. Perhaps changing the latter command's interface would be a 
> better choice; I'm not sure why it works that way.

What difference is that?  I must be missing something, since it sounds
like both commands use the argument to select absolute or relative
file names?

> Implementation difference: instead of going through (format "%S"), this 
> one uses shell-quote-argument. Is that good enough for Windows systems?

It depends on how the result will be used.  But maybe we should do the
same as dired-copy-filename-as-kill, just for consistency of the UX?

> And one more difference which we might want to see fixed:
> 
>    (When there's a single file, no quoting is done.)
> 
> It seems like, at least, in some usage scenarios users would prefer to 
> have copied filename not quoted. E.g. for subsequent use with find-file 
> or... something similar. I don't really use this command all that much, 
> so maybe I'm mistaken here, though.

There will always be cases when quoting gets in the way, but hopefully
they are rare.  The important thing, IMO, is that quoting is useful
for many/most uses of the file names in VC commands.  Is it?

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: ivan-p-sokolov <at> ya.ru, 64531 <at> debbugs.gnu.org
Subject: Re: bug#64531: [PATCH v2] Add new command vc-dir-copy-filename-as-kill
Date: Sun, 20 Aug 2023 13:26:02 +0300

On 20/08/2023 09:26, Eli Zaretskii wrote:
>> Date: Sun, 20 Aug 2023 02:33:59 +0300
>> Cc: 64531 <at> debbugs.gnu.org
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>>
>> The difference from dired-copy-filename-as-kill in handing of the prefix
>> argument looks unfortunate (as a principle), but I'd rather leave it for
>> you to judge. Perhaps changing the latter command's interface would be a
>> better choice; I'm not sure why it works that way.
> 
> What difference is that?  I must be missing something, since it sounds
> like both commands use the argument to select absolute or relative
> file names?

One of them mandates M-0 for that effect.

>> Implementation difference: instead of going through (format "%S"), this
>> one uses shell-quote-argument. Is that good enough for Windows systems?
> 
> It depends on how the result will be used.  But maybe we should do the
> same as dired-copy-filename-as-kill, just for consistency of the UX?

Maybe.

I imagine it might be used in a shell, but indeed there are different 
options.

>> And one more difference which we might want to see fixed:
>>
>>     (When there's a single file, no quoting is done.)
>>
>> It seems like, at least, in some usage scenarios users would prefer to
>> have copied filename not quoted. E.g. for subsequent use with find-file
>> or... something similar. I don't really use this command all that much,
>> so maybe I'm mistaken here, though.
> 
> There will always be cases when quoting gets in the way, but hopefully
> they are rare.  The important thing, IMO, is that quoting is useful
> for many/most uses of the file names in VC commands.  Is it?

When using 'M-!'? Maybe. I think consistency between 'w' commands is 
somewhat important, though.

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

From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
To: 64531 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: [PATCH v3] Add new commands for copying VC filenames
Date: Mon, 04 Sep 2023 23:08:02 +0300

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

Hi, Eli and Dmitry!

I tried to write documentation for the previous version of the command
and came to the conclusion that I crammed too much functionality into
it.  So I divided the DWIM command into command for copying name at
point (bound to w) and command for copying marked files (bound to * w).

I also added new command for copying VC related filenames from
everywhere.  An appropriate binding would be C-x v w, but I am not sure
if I should use a top-level binding for such an insignificant command.

Quoting and prefix arguments now copy `dired-copy-filename-as-kill'.

[0001-Add-new-commands-for-copying-VC-filenames.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
Cc: 64531 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: [PATCH v3] Add new commands for copying VC filenames
Date: Tue, 05 Sep 2023 14:01:56 +0300

> From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Mon, 04 Sep 2023 23:08:02 +0300
> 
> I tried to write documentation for the previous version of the command
> and came to the conclusion that I crammed too much functionality into
> it.  So I divided the DWIM command into command for copying name at
> point (bound to w) and command for copying marked files (bound to * w).
> 
> I also added new command for copying VC related filenames from
> everywhere.  An appropriate binding would be C-x v w, but I am not sure
> if I should use a top-level binding for such an insignificant command.
> 
> Quoting and prefix arguments now copy `dired-copy-filename-as-kill'.

Thanks, a few minor comments about the documentation parts:

> diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
> index 2dad70d3d13..8e7e2365f0a 100644
> --- a/doc/emacs/maintaining.texi
> +++ b/doc/emacs/maintaining.texi
> @@ -1388,6 +1388,13 @@ You can use this command to mark files that are in one of registered
>  states, including edited, added or removed.
>  (@code{vc-dir-mark-registered-files}).
>  
> +@findex vc-dir-copy-marked-files-as-kill
> +@item * w
> +This command copy names of marked files into the kill ring.  If prefix
                ^^^^
"copies"

> +argument is 0 names are absolute, with other prefix arguments names
> +are relative to the VC root directory.  Without prefix names are
> +relative to the VC buffer directory.

Our style is to describe the "normal" case first.  Like this:

  Normally, the copied file names are relative to the current buffer's
  default directory, but you can control that with the prefix
  argument.  With prefix argument of zero, the file names are copied
  in their absolute form; with any other value of prefix argument, the
  file names are relative to the root directory of the VCS repository.

> +@item w
> +Copy the name of the file at point into the kill ring.  If prefix
> +argument is 0 name is absolute, with other prefix arguments name is
> +relative to the VC root directory.  Without prefix name is relative to
> +the VC buffer directory.

This basically repeats the same text as above.  We usually first
describe the command in short, preferably in one sentence, and place
the detailed description further down.  Please try to follow these
conventions.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
Cc: 64531 <at> debbugs.gnu.org
Subject: Re: bug#64531: [PATCH v3] Add new commands for copying VC filenames
Date: Fri, 20 Oct 2023 02:39:24 +0300

On 05/09/2023 14:01, Eli Zaretskii wrote:
>> From: Ivan Sokolov<ivan-p-sokolov <at> ya.ru>
>> Cc: Eli Zaretskii<eliz <at> gnu.org>, Dmitry Gutov<dgutov <at> yandex.ru>
>> Date: Mon, 04 Sep 2023 23:08:02 +0300
>>
>> I tried to write documentation for the previous version of the command
>> and came to the conclusion that I crammed too much functionality into
>> it.  So I divided the DWIM command into command for copying name at
>> point (bound to w) and command for copying marked files (bound to * w).
>>
>> I also added new command for copying VC related filenames from
>> everywhere.  An appropriate binding would be C-x v w, but I am not sure
>> if I should use a top-level binding for such an insignificant command.
>>
>> Quoting and prefix arguments now copy `dired-copy-filename-as-kill'.
> Thanks, a few minor comments about the documentation parts:

Thank for the doc changes review.

The code looks good, FWIW.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64531 <at> debbugs.gnu.org, Ivan Sokolov <ivan-p-sokolov <at> ya.ru>,
 dgutov <at> yandex.ru
Subject: Re: bug#64531: [PATCH v3] Add new commands for copying VC filenames
Date: Wed, 12 Feb 2025 00:37:04 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Ivan Sokolov <ivan-p-sokolov <at> ya.ru>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Mon, 04 Sep 2023 23:08:02 +0300
>>
>> I tried to write documentation for the previous version of the command
>> and came to the conclusion that I crammed too much functionality into
>> it.  So I divided the DWIM command into command for copying name at
>> point (bound to w) and command for copying marked files (bound to * w).
>>
>> I also added new command for copying VC related filenames from
>> everywhere.  An appropriate binding would be C-x v w, but I am not sure
>> if I should use a top-level binding for such an insignificant command.
>>
>> Quoting and prefix arguments now copy `dired-copy-filename-as-kill'.
>
> Thanks, a few minor comments about the documentation parts:
>
>> diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
>> index 2dad70d3d13..8e7e2365f0a 100644
>> --- a/doc/emacs/maintaining.texi
>> +++ b/doc/emacs/maintaining.texi
>> @@ -1388,6 +1388,13 @@ You can use this command to mark files that are in one of registered
>>  states, including edited, added or removed.
>>  (@code{vc-dir-mark-registered-files}).
>>
>> +@findex vc-dir-copy-marked-files-as-kill
>> +@item * w
>> +This command copy names of marked files into the kill ring.  If prefix
>                 ^^^^
> "copies"
>
>> +argument is 0 names are absolute, with other prefix arguments names
>> +are relative to the VC root directory.  Without prefix names are
>> +relative to the VC buffer directory.
>
> Our style is to describe the "normal" case first.  Like this:
>
>   Normally, the copied file names are relative to the current buffer's
>   default directory, but you can control that with the prefix
>   argument.  With prefix argument of zero, the file names are copied
>   in their absolute form; with any other value of prefix argument, the
>   file names are relative to the root directory of the VCS repository.
>
>> +@item w
>> +Copy the name of the file at point into the kill ring.  If prefix
>> +argument is 0 name is absolute, with other prefix arguments name is
>> +relative to the VC root directory.  Without prefix name is relative to
>> +the VC buffer directory.
>
> This basically repeats the same text as above.  We usually first
> describe the command in short, preferably in one sentence, and place
> the detailed description further down.  Please try to follow these
> conventions.

Ivan, could you please send an updated patch based on Eli's comments
above?  Thanks in advance.

Previous Next

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