[PATCH v2 1/2] git-send-email: two new options: to-cover, cc-cover

[PATCH v2 1/2] git-send-email: two new options: to-cover, cc-cover

[Michael S. Tsirkin]

Allow extracting To/Cc addresses from cover letter.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 Documentation/git-send-email.txt | 12 ++++++++++++
 git-send-email.perl              | 16 ++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index f0e57a5..1733664 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -248,6 +248,18 @@ Automating
 	cc list. Default is the value of 'sendemail.signedoffbycc' configuration
 	value; if that is unspecified, default to --signed-off-by-cc.
 
+--[no-]cc-cover::
+	If this is set, emails found in Cc: headers in the cover letter are
+	added to the cc list for each email set. Default is the value of
+	'sendemail.cccover' configuration value; if that is unspecified,
+	default to --no-cc-cover.
+
+--[no-]to-cover::
+	If this is set, emails found in To: headers in the cover letter are
+	added to the to list for each email set. Default is the value of
+	'sendemail.tocover' configuration value; if that is unspecified,
+	default to --no-to-cover.
+
 --suppress-cc=<category>::
 	Specify an additional category of recipients to suppress the
 	auto-cc of:
diff --git a/git-send-email.perl b/git-send-email.perl
index 8bbfb84..11d9a46 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -80,6 +80,8 @@ git send-email [options] <file | directory | rev-list options >
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
     --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
     --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, all.
+    --[no-]cc-cover                * Email Cc: addresses in the cover letter.
+    --[no-]to-cover                * Email To: addresses in the cover letter.
     --[no-]signed-off-by-cc        * Send to Signed-off-by: addresses. Default on.
     --[no-]suppress-from           * Send to self. Default off.
     --[no-]chain-reply-to          * Chain In-Reply-To: fields. Default off.
@@ -195,6 +197,7 @@ sub do_edit {
 
 # Variables with corresponding config settings
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc);
+my ($cover_cc, $cover_to);
 my ($to_cmd, $cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
@@ -211,6 +214,8 @@ my %config_bool_settings = (
     "chainreplyto" => [\$chain_reply_to, 0],
     "suppressfrom" => [\$suppress_from, undef],
     "signedoffbycc" => [\$signed_off_by_cc, undef],
+    "cccover" => [\$cover_cc, undef],
+    "tocover" => [\$cover_to, undef],
     "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
     "validate" => [\$validate, 1],
     "multiedit" => [\$multiedit, undef],
@@ -302,6 +307,8 @@ my $rc = GetOptions("h" => \$help,
 		    "suppress-from!" => \$suppress_from,
 		    "suppress-cc=s" => \@suppress_cc,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
+		    "cc-cover|cc-cover!" => \$cover_cc,
+		    "to-cover|to-cover!" => \$cover_to,
 		    "confirm=s" => \$confirm,
 		    "dry-run" => \$dry_run,
 		    "envelope-sender=s" => \$envelope_sender,
@@ -1468,6 +1475,15 @@ foreach my $t (@files) {
 	@to = (@initial_to, @to);
 	@cc = (@initial_cc, @cc);
 
+	if ($message_num == 1) {
+		if (defined $cover_cc and $cover_cc) {
+			@initial_cc = @cc;
+		}
+		if (defined $cover_to and $cover_to) {
+			@initial_to = @to;
+		}
+	}
+
 	my $message_was_sent = send_message();
 
 	# set up for the next message
-- 
MST

[PATCH v2 2/2] test/send-email: add to-cover test

[Michael S. Tsirkin]

Does it work? I am not sure.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 t/t9001-send-email.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 3119c8c..3b17884 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1334,6 +1334,22 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '
 	test -n "$(ls msgtxt*)"
 '
 
+test_expect_success $PREREQ 'to-cover adds To to all mail' '
+	clean_fake_sendmail &&
+	rm -fr outdir &&
+	git format-patch --cover-letter -2 -o outdir &&
+	git send-email \
+	  --force \
+	  --from="Example <nobody@example.com>" \
+	  --to=nobody@example.com \
+	  --smtp-server="$(pwd)/fake.sendmail" \
+	  outdir/0002-*.patch \
+	  outdir/0000-*.patch \
+	  outdir/0001-*.patch \
+	  2>errors >out &&
+	! grep "SUBJECT HERE" errors &&
+	test -n "$(ls msgtxt*)"
+'
 test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
 	clean_fake_sendmail &&
 	echo "alias sbd  somebody@example.org" >.mailrc &&
-- 
MST

Re: [PATCH v2 1/2] git-send-email: two new options: to-cover, cc-cover

[Junio C Hamano]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> Allow extracting To/Cc addresses from cover letter.

Please say what you are doing with what you extract, which is the
more important part of the objective.  Extracting is merely a step
to achieve that.

s/.$/, to be used as To/Cc addresses of the remainder of the series./

or something.


I think this will be a very handy feature.

If you have a series *and* you bothered to add To/Cc to the cover
letter, it is likely that you want all the messages read by these
people [*1*].

> @@ -1468,6 +1475,15 @@ foreach my $t (@files) {
>  	@to = (@initial_to, @to);
>  	@cc = (@initial_cc, @cc);
>  
> +	if ($message_num == 1) {
> +		if (defined $cover_cc and $cover_cc) {
> +			@initial_cc = @cc;
> +		}
> +		if (defined $cover_to and $cover_to) {
> +			@initial_to = @to;
> +		}
> +	}
> +

What is stored away with this code to @initial_cc/to includes:

 - what was given to @initial_cc/to before ll.1468-1469
 - what was in @cc/to before ll.1468-1469

when we see the first message [*2*].  The former come from the
command line --to/--cc, and the latter comes from the header lines
of the first message.  Am I reading the code correctly?

If that is the case, I think the updated code makes sense.

Thanks.


[Footnote]

*1* Allowing this to be disabled is also a good thing this patch
    does.  A 100 patch series that does a tree-wide clean-up may
    have different set of people on To/Cc of individual patches, and
    you may want the union of them on To/Cc on the cover letter, so
    that a person may get the cover letter and a single patch that
    relates to his area of expertise without having to see the
    remainder.

*2* The first message may not necessarily be the cover letter.  Is
    there a reliable way to detect that?  The user may want to send
    out a series with only a few patches without any cover, and
    taking To/Cc from the [PATCH 1/3] and propagating them to the
    rest does not match what the documentation and the option name
    claim to do.

Re: [PATCH v2 2/2] test/send-email: add to-cover test

[Junio C Hamano]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> Does it work? I am not sure.

Then why was it sent here?

> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 3119c8c..3b17884 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1334,6 +1334,22 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '
>  	test -n "$(ls msgtxt*)"
>  '
>  
> +test_expect_success $PREREQ 'to-cover adds To to all mail' '
> +	clean_fake_sendmail &&
> +	rm -fr outdir &&
> +	git format-patch --cover-letter -2 -o outdir &&
> +	git send-email \
> +	  --force \
> +	  --from="Example <nobody@example.com>" \
> +	  --to=nobody@example.com \
> +	  --smtp-server="$(pwd)/fake.sendmail" \
> +	  outdir/0002-*.patch \
> +	  outdir/0000-*.patch \
> +	  outdir/0001-*.patch \
> +	  2>errors >out &&
> +	! grep "SUBJECT HERE" errors &&
> +	test -n "$(ls msgtxt*)"
> +'

Is this a copy of an existing "--force can disable the safety to
catch a mistake to send a cover letter template without any update"?

How are you checking if you are propagating to/cc from the cover to
other messages with this test?

Puzzled.

Re: [PATCH v2 1/2] git-send-email: two new options: to-cover, cc-cover

[Eric Sunshine]

On Thu, Apr 3, 2014 at 2:14 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> Allow extracting To/Cc addresses from cover letter.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  Documentation/git-send-email.txt | 12 ++++++++++++
>  git-send-email.perl              | 16 ++++++++++++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index f0e57a5..1733664 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -248,6 +248,18 @@ Automating
>         cc list. Default is the value of 'sendemail.signedoffbycc' configuration
>         value; if that is unspecified, default to --signed-off-by-cc.
>
> +--[no-]cc-cover::
> +       If this is set, emails found in Cc: headers in the cover letter are
> +       added to the cc list for each email set. Default is the value of

s/email set/email sent/

> +       'sendemail.cccover' configuration value; if that is unspecified,
> +       default to --no-cc-cover.
> +
> +--[no-]to-cover::
> +       If this is set, emails found in To: headers in the cover letter are
> +       added to the to list for each email set. Default is the value of

Ditto.

> +       'sendemail.tocover' configuration value; if that is unspecified,
> +       default to --no-to-cover.
> +
>  --suppress-cc=<category>::
>         Specify an additional category of recipients to suppress the
>         auto-cc of:
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 8bbfb84..11d9a46 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -80,6 +80,8 @@ git send-email [options] <file | directory | rev-list options >
>      --to-cmd                <str>  * Email To: via `<str> \$patch_path`
>      --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
>      --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, all.
> +    --[no-]cc-cover                * Email Cc: addresses in the cover letter.
> +    --[no-]to-cover                * Email To: addresses in the cover letter.
>      --[no-]signed-off-by-cc        * Send to Signed-off-by: addresses. Default on.
>      --[no-]suppress-from           * Send to self. Default off.
>      --[no-]chain-reply-to          * Chain In-Reply-To: fields. Default off.
> @@ -195,6 +197,7 @@ sub do_edit {
>
>  # Variables with corresponding config settings
>  my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc);
> +my ($cover_cc, $cover_to);
>  my ($to_cmd, $cc_cmd);
>  my ($smtp_server, $smtp_server_port, @smtp_server_options);
>  my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
> @@ -211,6 +214,8 @@ my %config_bool_settings = (
>      "chainreplyto" => [\$chain_reply_to, 0],
>      "suppressfrom" => [\$suppress_from, undef],
>      "signedoffbycc" => [\$signed_off_by_cc, undef],
> +    "cccover" => [\$cover_cc, undef],
> +    "tocover" => [\$cover_to, undef],
>      "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
>      "validate" => [\$validate, 1],
>      "multiedit" => [\$multiedit, undef],
> @@ -302,6 +307,8 @@ my $rc = GetOptions("h" => \$help,
>                     "suppress-from!" => \$suppress_from,
>                     "suppress-cc=s" => \@suppress_cc,
>                     "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
> +                   "cc-cover|cc-cover!" => \$cover_cc,
> +                   "to-cover|to-cover!" => \$cover_to,
>                     "confirm=s" => \$confirm,
>                     "dry-run" => \$dry_run,
>                     "envelope-sender=s" => \$envelope_sender,
> @@ -1468,6 +1475,15 @@ foreach my $t (@files) {
>         @to = (@initial_to, @to);
>         @cc = (@initial_cc, @cc);
>
> +       if ($message_num == 1) {
> +               if (defined $cover_cc and $cover_cc) {
> +                       @initial_cc = @cc;
> +               }
> +               if (defined $cover_to and $cover_to) {
> +                       @initial_to = @to;
> +               }
> +       }
> +
>         my $message_was_sent = send_message();
>
>         # set up for the next message
> --
> MST
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] git-send-email: two new options: to-cover, cc-cover

[Michael S. Tsirkin]

On Thu, Apr 03, 2014 at 11:31:51AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > Allow extracting To/Cc addresses from cover letter.
> 
> Please say what you are doing with what you extract, which is the
> more important part of the objective.  Extracting is merely a step
> to achieve that.
> 
> s/.$/, to be used as To/Cc addresses of the remainder of the series./
> 
> or something.
> 

thanks, I did that in the new version.


> I think this will be a very handy feature.
> 
> If you have a series *and* you bothered to add To/Cc to the cover
> letter, it is likely that you want all the messages read by these
> people [*1*].
> 
> > @@ -1468,6 +1475,15 @@ foreach my $t (@files) {
> >  	@to = (@initial_to, @to);
> >  	@cc = (@initial_cc, @cc);
> >  
> > +	if ($message_num == 1) {
> > +		if (defined $cover_cc and $cover_cc) {
> > +			@initial_cc = @cc;
> > +		}
> > +		if (defined $cover_to and $cover_to) {
> > +			@initial_to = @to;
> > +		}
> > +	}
> > +
> 
> What is stored away with this code to @initial_cc/to includes:
> 
>  - what was given to @initial_cc/to before ll.1468-1469
>  - what was in @cc/to before ll.1468-1469
> 
> when we see the first message [*2*].  The former come from the
> command line --to/--cc, and the latter comes from the header lines
> of the first message.  Am I reading the code correctly?

Exactly.

> If that is the case, I think the updated code makes sense.
> 
> Thanks.
> 
> 
> [Footnote]
> 
> *1* Allowing this to be disabled is also a good thing this patch
>     does.  A 100 patch series that does a tree-wide clean-up may
>     have different set of people on To/Cc of individual patches, and
>     you may want the union of them on To/Cc on the cover letter, so
>     that a person may get the cover letter and a single patch that
>     relates to his area of expertise without having to see the
>     remainder.
> 
> *2* The first message may not necessarily be the cover letter.  Is
>     there a reliable way to detect that?


>  The user may want to send
>     out a series with only a few patches without any cover, and
>     taking To/Cc from the [PATCH 1/3] and propagating them to the
>     rest does not match what the documentation and the option name
>     claim to do.

Two things that come to mind:
	- check that subject has 0000/
		Needs some manual parsing, I don't like this much
	- check that there's no patch
		We could try running git mailinfo but it might give
		false negatives if cover letter happens to have
			---
			diff a/foo b/bar
		within it.
		Worth worrying about?

For now I simply updated the documentation.

-- 
MST