{"thread":{"id":"51551","subject":"[PATCH] send-email: Ask if a patch should be sent twice","startedAt":"2019-07-30T00:13:57Z","lastAt":"2019-07-30T12:50:10Z","messageCount":3,"participants":["Dmitry Safonov","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"379566","messageId":"20190730001352.7477-1-dima@arista.com","threadId":"51551","inReplyTo":null,"subject":"[PATCH] send-email: Ask if a patch should be sent twice","fromName":"Dmitry Safonov","fromEmail":"dima@arista.com","sentAt":"2019-07-30T00:13:52Z","receivedAt":"2019-07-30T00:13:57Z","isPatch":true,"sender":{"key":"dima@arista.com","avatar":null},"body":"I was almost certain that git won't let me send the same patch twice,\nbut today I've managed to double-send a directory by a mistake:\n\tgit send-email --to linux-kernel@vger.kernel.org /tmp/timens/\n\t    --cc 'Dmitry Safonov <0x7f454c46@gmail.com>' /tmp/timens/`\n\n[I haven't noticed that I put the directory twice ^^]\n\nPrevent this shipwreck from happening again by asking if a patch\nis sent multiple times on purpose.\n\nlink: https://lkml.kernel.org/r/4d53ebc7-d5b2-346e-c383-606401d19d3a@gmail.com\nCc: Andrei Vagin <avagin@openvz.org>\nSigned-off-by: Dmitry Safonov <dima@arista.com>\n---\n git-send-email.perl | 23 ++++++++++++++++++++++-\n 1 file changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5f92c89c1c1b..0caafc104478 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -33,6 +33,7 @@\n use Net::Domain ();\n use Net::SMTP ();\n use Git::LoadCPAN::Mail::Address;\n+use experimental 'smartmatch';\n \n Getopt::Long::Configure qw/ pass_through /;\n \n@@ -658,6 +659,17 @@ sub is_format_patch_arg {\n \t}\n }\n \n+sub send_file_twice {\n+\tmy $f = shift;\n+\t$_ = ask(__(\"Patch $f will be sent twice, continue? [y]/n \"),\n+\t\tdefault => \"y\",\n+\t\tvalid_re => qr/^(?:yes|y|no|n)/i);\n+\tif (/^n/i) {\n+\t\tcleanup_compose_files();\n+\t\texit(0);\n+\t}\n+}\n+\n # Now that all the defaults are set, process the rest of the command line\n # arguments and collect up the files that need to be processed.\n my @rev_list_opts;\n@@ -669,10 +681,19 @@ sub is_format_patch_arg {\n \t\topendir my $dh, $f\n \t\t\tor die sprintf(__(\"Failed to opendir %s: %s\"), $f, $!);\n \n-\t\tpush @files, grep { -f $_ } map { catfile($f, $_) }\n+\t\tmy @new_files = grep { -f $_ } map { catfile($f, $_) }\n \t\t\t\tsort readdir $dh;\n+\t\tforeach my $nfile (@new_files) {\n+\t\t\tif ($nfile ~~ @files) {\n+\t\t\t\tsend_file_twice($nfile);\n+\t\t\t}\n+\t\t}\n+\t\tpush @files, @new_files;\n \t\tclosedir $dh;\n \t} elsif ((-f $f or -p $f) and !is_format_patch_arg($f)) {\n+\t\tif ($f ~~ @files) {\n+\t\t\tsend_file_twice($f);\n+\t\t}\n \t\tpush @files, $f;\n \t} else {\n \t\tpush @rev_list_opts, $f;\n-- \n2.22.0\n\n"},{"id":"379574","messageId":"87r26768lc.fsf@evledraar.gmail.com","threadId":"51551","inReplyTo":"20190730001352.7477-1-dima@arista.com","subject":"Re: [PATCH] send-email: Ask if a patch should be sent twice","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-30T11:54:39Z","receivedAt":"2019-07-30T11:54:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jul 30 2019, Dmitry Safonov wrote:\n\n> I was almost certain that git won't let me send the same patch twice,\n> but today I've managed to double-send a directory by a mistake:\n> \tgit send-email --to linux-kernel@vger.kernel.org /tmp/timens/\n> \t    --cc 'Dmitry Safonov <0x7f454c46@gmail.com>' /tmp/timens/`\n>\n> [I haven't noticed that I put the directory twice ^^]\n>\n> Prevent this shipwreck from happening again by asking if a patch\n> is sent multiple times on purpose.\n>\n> link: https://lkml.kernel.org/r/4d53ebc7-d5b2-346e-c383-606401d19d3a@gmail.com\n> Cc: Andrei Vagin <avagin@openvz.org>\n> Signed-off-by: Dmitry Safonov <dima@arista.com>\n> ---\n>  git-send-email.perl | 23 ++++++++++++++++++++++-\n>  1 file changed, 22 insertions(+), 1 deletion(-)\n\nThere's tests for send-email in t/t9001-send-email.sh. See if what\nyou're adding can have a test added, seems simple enough in this case.\n\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 5f92c89c1c1b..0caafc104478 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -33,6 +33,7 @@\n>  use Net::Domain ();\n>  use Net::SMTP ();\n>  use Git::LoadCPAN::Mail::Address;\n> +use experimental 'smartmatch';\n\nWe depend on Perl 5.8, this bumps the requirenment to 5.10. Aside from\nthat ~~ is its own can of worms in Perl and is best avoided.\n\n>  Getopt::Long::Configure qw/ pass_through /;\n>\n> @@ -658,6 +659,17 @@ sub is_format_patch_arg {\n>  \t}\n>  }\n>\n> +sub send_file_twice {\n> +\tmy $f = shift;\n> +\t$_ = ask(__(\"Patch $f will be sent twice, continue? [y]/n \"),\n\nThese cases with a default should have \"Y/n\", not \"y/n\". See other\nexpamples in the file.\n\n> +\t\tdefault => \"y\",\n> +\t\tvalid_re => qr/^(?:yes|y|no|n)/i);\n> +\tif (/^n/i) {\n> +\t\tcleanup_compose_files();\n> +\t\texit(0);\n\nExit if we have just one of these? More on that later...\n\n> +\t}\n> +}\n> +\n>  # Now that all the defaults are set, process the rest of the command line\n>  # arguments and collect up the files that need to be processed.\n>  my @rev_list_opts;\n> @@ -669,10 +681,19 @@ sub is_format_patch_arg {\n>  \t\topendir my $dh, $f\n>  \t\t\tor die sprintf(__(\"Failed to opendir %s: %s\"), $f, $!);\n>\n> -\t\tpush @files, grep { -f $_ } map { catfile($f, $_) }\n> +\t\tmy @new_files = grep { -f $_ } map { catfile($f, $_) }\n>  \t\t\t\tsort readdir $dh;\n> +\t\tforeach my $nfile (@new_files) {\n> +\t\t\tif ($nfile ~~ @files) {\n> +\t\t\t\tsend_file_twice($nfile);\n> +\t\t\t}\n\nOne non-smartmatch idiom for this is:\n\n    my %seen;\n    for my $file (@files) {\n        if ($seen{$file}++) { ...}\n    }\n\nOr:\n\n    my %seen;\n    my @dupes = grep { $seen{$_}++ } @files;\n\n> +\t\t}\n> +\t\tpush @files, @new_files;\n>  \t\tclosedir $dh;\n>  \t} elsif ((-f $f or -p $f) and !is_format_patch_arg($f)) {\n> +\t\tif ($f ~~ @files) {\n> +\t\t\tsend_file_twice($f);\n> +\t\t}\n>  \t\tpush @files, $f;\n\n...but picking up the comment above, I'd expect this to be in the \"if\n($validate)\" block below or something similar, seems like this fits\nright in with --validate.\n\nThen you can also ask \"do you want to send this set of patches twice\n<full list>?\".\n\nNow the user is asked a file-at-a-time.\n\n>  \t} else {\n>  \t\tpush @rev_list_opts, $f;\n"},{"id":"379575","messageId":"59e47154-5b07-4ef0-9883-dc3f53b31fc2@arista.com","threadId":"51551","inReplyTo":"87r26768lc.fsf@evledraar.gmail.com","subject":"Re: [PATCH] send-email: Ask if a patch should be sent twice","fromName":"Dmitry Safonov","fromEmail":"dima@arista.com","sentAt":"2019-07-30T12:50:05Z","receivedAt":"2019-07-30T12:50:10Z","isPatch":true,"sender":{"key":"dima@arista.com","avatar":null},"body":"On 7/30/19 12:54 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Tue, Jul 30 2019, Dmitry Safonov wrote:\n> \n>> I was almost certain that git won't let me send the same patch twice,\n>> but today I've managed to double-send a directory by a mistake:\n>> \tgit send-email --to linux-kernel@vger.kernel.org /tmp/timens/\n>> \t    --cc 'Dmitry Safonov <0x7f454c46@gmail.com>' /tmp/timens/`\n>>\n>> [I haven't noticed that I put the directory twice ^^]\n>>\n>> Prevent this shipwreck from happening again by asking if a patch\n>> is sent multiple times on purpose.\n>>\n>> link: https://lkml.kernel.org/r/4d53ebc7-d5b2-346e-c383-606401d19d3a@gmail.com\n>> Cc: Andrei Vagin <avagin@openvz.org>\n>> Signed-off-by: Dmitry Safonov <dima@arista.com>\n>> ---\n>>  git-send-email.perl | 23 ++++++++++++++++++++++-\n>>  1 file changed, 22 insertions(+), 1 deletion(-)\n> \n> There's tests for send-email in t/t9001-send-email.sh. See if what\n> you're adding can have a test added, seems simple enough in this case.\n\nI wasn't sure if that needs a test or some `--send-them-twice` option.\nDecided to send it early.. Will add a test.\n\n> \n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index 5f92c89c1c1b..0caafc104478 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -33,6 +33,7 @@\n>>  use Net::Domain ();\n>>  use Net::SMTP ();\n>>  use Git::LoadCPAN::Mail::Address;\n>> +use experimental 'smartmatch';\n> \n> We depend on Perl 5.8, this bumps the requirenment to 5.10. Aside from\n> that ~~ is its own can of worms in Perl and is best avoided.\n\nYeah, I'm not very into Perl and Stackoverflow *blush* suggested to use\n~~. Will drop.\n\n> \n>>  Getopt::Long::Configure qw/ pass_through /;\n>>\n>> @@ -658,6 +659,17 @@ sub is_format_patch_arg {\n>>  \t}\n>>  }\n>>\n>> +sub send_file_twice {\n>> +\tmy $f = shift;\n>> +\t$_ = ask(__(\"Patch $f will be sent twice, continue? [y]/n \"),\n> \n> These cases with a default should have \"Y/n\", not \"y/n\". See other\n> expamples in the file.\n\nOk.\n\n> \n>> +\t\tdefault => \"y\",\n>> +\t\tvalid_re => qr/^(?:yes|y|no|n)/i);\n>> +\tif (/^n/i) {\n>> +\t\tcleanup_compose_files();\n>> +\t\texit(0);\n> \n> Exit if we have just one of these? More on that later...\n> \n>> +\t}\n>> +}\n>> +\n>>  # Now that all the defaults are set, process the rest of the command line\n>>  # arguments and collect up the files that need to be processed.\n>>  my @rev_list_opts;\n>> @@ -669,10 +681,19 @@ sub is_format_patch_arg {\n>>  \t\topendir my $dh, $f\n>>  \t\t\tor die sprintf(__(\"Failed to opendir %s: %s\"), $f, $!);\n>>\n>> -\t\tpush @files, grep { -f $_ } map { catfile($f, $_) }\n>> +\t\tmy @new_files = grep { -f $_ } map { catfile($f, $_) }\n>>  \t\t\t\tsort readdir $dh;\n>> +\t\tforeach my $nfile (@new_files) {\n>> +\t\t\tif ($nfile ~~ @files) {\n>> +\t\t\t\tsend_file_twice($nfile);\n>> +\t\t\t}\n> \n> One non-smartmatch idiom for this is:\n> \n>     my %seen;\n>     for my $file (@files) {\n>         if ($seen{$file}++) { ...}\n>     }\n> \n> Or:\n> \n>     my %seen;\n>     my @dupes = grep { $seen{$_}++ } @files;\n> \n>> +\t\t}\n>> +\t\tpush @files, @new_files;\n>>  \t\tclosedir $dh;\n>>  \t} elsif ((-f $f or -p $f) and !is_format_patch_arg($f)) {\n>> +\t\tif ($f ~~ @files) {\n>> +\t\t\tsend_file_twice($f);\n>> +\t\t}\n>>  \t\tpush @files, $f;\n> \n> ...but picking up the comment above, I'd expect this to be in the \"if\n> ($validate)\" block below or something similar, seems like this fits\n> right in with --validate.\n\nBy default it's on - sounds good to me.\n\n> Then you can also ask \"do you want to send this set of patches twice\n> <full list>?\".\n> \n> Now the user is asked a file-at-a-time.\n\nOk, thanks for the suggestions.\n\n-- \n          Dmitry\n"}]}