[PATCH] send-email: handle multiple Cc addresses when reading mbox message

Subsystems: the rest

DORMANTno replies

15 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH] send-email: handle multiple Cc addresses when reading mbox message

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

From: Jay Soffian <redacted>

When git format-patch is given multiple --cc arguments, it generates a
Cc header that looks like:

 Cc: first@example.com,
     second@example.com,
     third@example.com

Before this commit, send-email was unable to handle such a message.
First, it didn't know how to handle a header split across multiple
lines. Second, it couldn't handle multiple comma-separated addresses,
even if they were provided all on a single Cc: line.

This patch teaches it to unfold header lines by pre-processing the
header before extracting any of its fields. It also teaches it to split
the Cc line using Mail::Address if available, otherwise it just splits
on a simple \s*,\s* regular expression.

Also, conditionally use Mail::Address to handle the response to the "Who
should the emails be sent to?" prompt.

Signed-off-by: Jay Soffian <redacted>
---
This message which you are reading was generated with:
$ git format-patch --cc="Ryan Anderson [off-list ref]" \
  --cc=gitster@pobox.com -s -1

And sent with:
$ git send-email --from 'Jay Soffian [off-list ref]' \
  --to git@vger.kernel.org  --cc 'Jay Soffian [off-list ref]'

Before this patch, send-email would not have sent it properly. :-)

j.

 git-send-email.perl   |  150 +++++++++++++++++++++++++++---------------------
 t/t9001-send-email.sh |   22 +++++--
 2 files changed, 99 insertions(+), 73 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 77ca8fe..cde294c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -126,6 +126,7 @@ sub format_2822_time {
 }
 
 my $have_email_valid = eval { require Email::Valid; 1 };
+my $have_mail_address = eval { require Mail::Address; 1 };
 my $smtp;
 my $auth;
 
@@ -360,6 +361,14 @@ foreach my $entry (@bcclist) {
 	die "Comma in --bcclist entry: $entry'\n" unless $entry !~ m/,/;
 }
 
+sub parse_address_line {
+	if ($have_mail_address) {
+		return map { $_->format } Mail::Address->parse($_[0]);
+	} else {
+		return split_addrs($_[0]);
+	}
+}
+
 sub split_addrs {
 	return quotewords('\s*,\s*', 1, @_);
 }
@@ -593,7 +602,7 @@ if (!@to) {
 	}
 
 	my $to = $_;
-	push @to, split_addrs($to);
+	push @to, parse_address_line($to);
 	$prompting++;
 }
 
@@ -920,88 +929,97 @@ foreach my $t (@files) {
 	@cc = @initial_cc;
 	@xh = ();
 	my $input_format = undef;
-	my $header_done = 0;
+	my @header = ();
 	$message = "";
+	# First unfold multiline header fields
 	while(<F>) {
-		if (!$header_done) {
-			if (/^From /) {
-				$input_format = 'mbox';
-				next;
+		last if /^\s*$/;
+		if (/^\s+\S/ and @header) {
+			chomp($header[$#header]);
+			s/^\s+/ /;
+			$header[$#header] .= $_;
+	    } else {
+			push(@header, $_);
+		}
+	}
+	# Now parse the header
+	foreach(@header) {
+		if (/^From /) {
+			$input_format = 'mbox';
+			next;
+		}
+		chomp;
+		if (!defined $input_format && /^[-A-Za-z]+:\s/) {
+			$input_format = 'mbox';
+		}
+
+		if (defined $input_format && $input_format eq 'mbox') {
+			if (/^Subject:\s+(.*)$/) {
+				$subject = $1;
 			}
-			chomp;
-			if (!defined $input_format && /^[-A-Za-z]+:\s/) {
-				$input_format = 'mbox';
+			elsif (/^From:\s+(.*)$/) {
+				($author, $author_encoding) = unquote_rfc2047($1);
+				next if ($suppress_cc{'author'});
+				printf("(mbox) Adding cc: %s from line '%s'\n",
+					$1, $_) unless $quiet;
+				push @cc, $1;
 			}
-
-			if (defined $input_format && $input_format eq 'mbox') {
-				if (/^Subject:\s+(.*)$/) {
-					$subject = $1;
-
-				} elsif (/^(Cc|From):\s+(.*)$/) {
-					if (unquote_rfc2047($2) eq $sender) {
+			elsif (/^Cc:\s+(.*)$/) {
+				foreach my $addr (parse_address_line($1)) {
+					if (unquote_rfc2047($addr) eq $sender) {
 						next if ($suppress_cc{'self'});
-					}
-					elsif ($1 eq 'From') {
-						($author, $author_encoding)
-						  = unquote_rfc2047($2);
-						next if ($suppress_cc{'author'});
 					} else {
 						next if ($suppress_cc{'cc'});
 					}
 					printf("(mbox) Adding cc: %s from line '%s'\n",
-						$2, $_) unless $quiet;
-					push @cc, $2;
+						$addr, $_) unless $quiet;
+					push @cc, $addr;
 				}
-				elsif (/^Content-type:/i) {
-					$has_content_type = 1;
-					if (/charset="?([^ "]+)/) {
-						$body_encoding = $1;
-					}
-					push @xh, $_;
-				}
-				elsif (/^Message-Id: (.*)/i) {
-					$message_id = $1;
-				}
-				elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) {
-					push @xh, $_;
-				}
-
-			} else {
-				# In the traditional
-				# "send lots of email" format,
-				# line 1 = cc
-				# line 2 = subject
-				# So let's support that, too.
-				$input_format = 'lots';
-				if (@cc == 0 && !$suppress_cc{'cc'}) {
-					printf("(non-mbox) Adding cc: %s from line '%s'\n",
-						$_, $_) unless $quiet;
-
-					push @cc, $_;
-
-				} elsif (!defined $subject) {
-					$subject = $_;
+			}
+			elsif (/^Content-type:/i) {
+				$has_content_type = 1;
+				if (/charset="?([^ "]+)/) {
+					$body_encoding = $1;
 				}
+				push @xh, $_;
 			}
-
-			# A whitespace line will terminate the headers
-			if (m/^\s*$/) {
-				$header_done = 1;
+			elsif (/^Message-Id: (.*)/i) {
+				$message_id = $1;
 			}
+			elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) {
+				push @xh, $_;
+			}
+
 		} else {
-			$message .=  $_;
-			if (/^(Signed-off-by|Cc): (.*)$/i) {
-				next if ($suppress_cc{'sob'});
-				chomp;
-				my $c = $2;
-				chomp $c;
-				next if ($c eq $sender and $suppress_cc{'self'});
-				push @cc, $c;
-				printf("(sob) Adding cc: %s from line '%s'\n",
-					$c, $_) unless $quiet;
+			# In the traditional
+			# "send lots of email" format,
+			# line 1 = cc
+			# line 2 = subject
+			# So let's support that, too.
+			$input_format = 'lots';
+			if (@cc == 0 && !$suppress_cc{'cc'}) {
+				printf("(non-mbox) Adding cc: %s from line '%s'\n",
+					$_, $_) unless $quiet;
+				push @cc, $_;
+			} elsif (!defined $subject) {
+				$subject = $_;
 			}
 		}
 	}
+	# Now parse the message body
+	while(<F>) {
+		$message .=  $_;
+		if (/^(Signed-off-by|Cc): (.*)$/i) {
+			next if ($suppress_cc{'sob'});
+			chomp;
+			my $c = $2;
+			chomp $c;
+			next if ($c eq $sender and $suppress_cc{'self'});
+			push @cc, $c;
+			printf("(sob) Adding cc: %s from line '%s'\n",
+				$c, $_) unless $quiet;
+		}
+	}
 	close F;
 
 	if (defined $cc_cmd && !$suppress_cc{'cccmd'}) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index cb3d183..da54835 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -32,7 +32,7 @@ clean_fake_sendmail() {
 }
 
 test_expect_success 'Extract patches' '
-    patches=`git format-patch -n HEAD^1`
+    patches=`git format-patch --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1`
 '
 
 test_expect_success 'Send patches' '
@@ -42,6 +42,8 @@ test_expect_success 'Send patches' '
 cat >expected <<\EOF
 !nobody@example.com!
 !author@example.com!
+!one@example.com!
+!two@example.com!
 EOF
 test_expect_success \
     'Verify commandline' \
@@ -50,13 +52,15 @@ test_expect_success \
 cat >expected-show-all-headers <<\EOF
 0001-Second.patch
 (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
 Dry-OK. Log says:
 Server: relay.example.com
 MAIL FROM:<from@example.com>
-RCPT TO:<to@example.com>,<cc@example.com>,<author@example.com>,<bcc@example.com>
+RCPT TO:<to@example.com>,<cc@example.com>,<author@example.com>,<one@example.com>,<two@example.com>,<bcc@example.com>
 From: Example <from@example.com>
 To: to@example.com
-Cc: cc@example.com, A <author@example.com>
+Cc: cc@example.com, A <author@example.com>, One <one@example.com>, two@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
@@ -170,13 +174,15 @@ test_expect_success 'second message is patch' '
 cat >expected-show-all-headers <<\EOF
 0001-Second.patch
 (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
 Dry-OK. Log says:
 Server: relay.example.com
 MAIL FROM:<from@example.com>
-RCPT TO:<to@example.com>,<cc@example.com>,<author@example.com>
+RCPT TO:<to@example.com>,<cc@example.com>,<author@example.com>,<one@example.com>,<two@example.com>
 From: Example <from@example.com>
 To: to@example.com
-Cc: cc@example.com, A <author@example.com>
+Cc: cc@example.com, A <author@example.com>, One <one@example.com>, two@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
@@ -203,13 +209,15 @@ test_expect_success 'sendemail.cc set' '
 cat >expected-show-all-headers <<\EOF
 0001-Second.patch
 (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
 Dry-OK. Log says:
 Server: relay.example.com
 MAIL FROM:<from@example.com>
-RCPT TO:<to@example.com>,<author@example.com>
+RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>
 From: Example <from@example.com>
 To: to@example.com
-Cc: A <author@example.com>
+Cc: A <author@example.com>, One <one@example.com>, two@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
-- 
1.6.2.rc0.67.g77afc

Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message

From: Thomas Rast <hidden>
Date: 2016-06-15 22:46:10

Jay Soffian wrote:
-			if (/^(Signed-off-by|Cc): (.*)$/i) {
-				next if ($suppress_cc{'sob'});
[...]
+		if (/^(Signed-off-by|Cc): (.*)$/i) {
+			next if ($suppress_cc{'sob'});
Doesn't this actually look like a long-standing send-email bug?  Since
6564828 (git-send-email: Generalize auto-cc recipient mechanism.,
2007-12-25) they should go in separate categories, but the above lines
were just translated from the old $signed_off_cc setting.  It seems
they should distinguish between SOB and Cc.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

On Fri, Feb 13, 2009 at 6:32 PM, Thomas Rast [off-list ref] wrote:
Jay Soffian wrote:
quoted
-                     if (/^(Signed-off-by|Cc): (.*)$/i) {
-                             next if ($suppress_cc{'sob'});
[...]
quoted
+             if (/^(Signed-off-by|Cc): (.*)$/i) {
+                     next if ($suppress_cc{'sob'});
Doesn't this actually look like a long-standing send-email bug?  Since
6564828 (git-send-email: Generalize auto-cc recipient mechanism.,
2007-12-25) they should go in separate categories, but the above lines
were just translated from the old $signed_off_cc setting.  It seems
they should distinguish between SOB and Cc.
I think it would be nice if send-email had different settings for
extracting Cc lines and extracting SOB lines from the message body,
but I don't call that a bug, but rather an enhancement request.

And yes, I noticed it when I went to send this patch because
originally I had "Cc: ..." in the commit message on column 0, which
send-email annoyingly picked up. I just changed my commit message to
move the "Cc:" to column 1.

Nonetheless, it is independent of this patch.

j.

Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

On Fri, Feb 13, 2009 at 6:39 PM, Jay Soffian [off-list ref] wrote:
I think it would be nice if send-email had different settings for
extracting Cc lines and extracting SOB lines from the message body,
but I don't call that a bug, but rather an enhancement request.
Oh, I see it does now that I looked at that commit. Hmphf. I'll fix
that on-top of the patch I just sent.

j.

Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message

From: Jeff King <hidden>
Date: 2016-06-15 22:46:10

On Fri, Feb 13, 2009 at 06:05:13PM -0500, Jay Soffian wrote:
This patch teaches it to unfold header lines by pre-processing the
header before extracting any of its fields. It also teaches it to split
the Cc line using Mail::Address if available, otherwise it just splits
on a simple \s*,\s* regular expression.
Ugh. This is not in any way a comment on your patch, but it is really
painful to see time and again send-email bugs like this that could be
solved by using the wealth of existing perl modules.

I think send-email is stuck somewhere between "here is the bare minimum
to send some patches, and we don't want to require any fancy
dependencies" and "a full-fledged replacement for my MUA when dealing
with patches".

The approach of "use this module if we have it, otherwise do something
quick and dirty" might be the best approach. But I do wonder if the
"quick and dirty" code path will end up buggy due to lack of use, and/or
be surprising for people who expect send-email to be more thorough.

The other alternatives are to keep fixing bugs and improving
incrementally until it converges on being an actual MUA, or to simply
start a rewrite based on reasonable CPAN modules.

I don't know what the right solution is. I am certainly not volunteering
to rewrite; I am very happy just using my actual MUA to send patches. :)

-Peff

Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

On Fri, Feb 13, 2009 at 7:31 PM, Jeff King [off-list ref] wrote:
I don't know what the right solution is. I am certainly not volunteering
to rewrite; I am very happy just using my actual MUA to send patches. :)
It is quite ugly, and I have thought about rewriting it.

But to be quite honest, I have become very discouraged over the last
two weeks about submitting patches. What seems at first to be a simple
change has required multiple revisions over frankly what feels to me
like nit-picking.

It goes like this:

Me: Notice wart
Me: Cut wart off (i.e. submit patch)
Reviewer: Sorry, you didn't cut that wart off to my standards, unless
you can do it better, I'd rather have the wart.

I dunno, maybe I'm too sensitive and don't have the fortitude for
contributing to git.

Or maybe I'm just venting on a Friday.

j.

Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message

From: Jeff King <hidden>
Date: 2016-06-15 22:46:10

On Fri, Feb 13, 2009 at 07:42:16PM -0500, Jay Soffian wrote:
On Fri, Feb 13, 2009 at 7:31 PM, Jeff King [off-list ref] wrote:
quoted
I don't know what the right solution is. I am certainly not volunteering
to rewrite; I am very happy just using my actual MUA to send patches. :)
It is quite ugly, and I have thought about rewriting it.

But to be quite honest, I have become very discouraged over the last
two weeks about submitting patches. What seems at first to be a simple
change has required multiple revisions over frankly what feels to me
like nit-picking.
I hope it wasn't my message in this thread that sent you over the edge.
I really did mean the "this isn't a comment on your patch" but was just
musing out loud over what the right direction for send-email is in
general.
Me: Notice wart
Me: Cut wart off (i.e. submit patch)
Reviewer: Sorry, you didn't cut that wart off to my standards, unless
you can do it better, I'd rather have the wart.

I dunno, maybe I'm too sensitive and don't have the fortitude for
contributing to git.

Or maybe I'm just venting on a Friday.
I hope just venting. I think the warts you've been cutting off have been
improving git.

One of the things I have seen happening with your patches is something
like:

  Jay: Here's a patch.
  Reviewer: Good, but it should also do X to be complete.
              or
            Good, but there is one corner case where the correct
            behavior is unclear [queue endless discussion, production of
            several versions of the patch doing different behavior, or
            realization that some of the behavior would be an order of
            magnitude more work to implement]

Those sorts of reviews are good for helping git in the long run, and I
think reviewers are often thinking in terms of git's overall
progression. But I think it is also OK sometimes as a patch submitter to
say "This series scratches my itch, and I am done for now." And maybe
somebody else picks it up and builds on it. Or maybe you do, but weeks
or months later. Or maybe nobody does, because it turns out that nobody
really cares about that corner case in practice.

Which is really just "perfect is the enemy of the good" in another form,
and recognizing that git's development is incremental.

And all of that isn't to say there aren't plenty of patches that should
be rejected for incompleteness or lack of handling corner cases. Many
times an incomplete solution is worse than no solution at all. So there
is some judgement required about whether the changes are a net positive
and a net negative.

But I think it is up to the submitter to decide how far he wants to take
a given topic (and at what rate), and to say "OK, I'll go implement
that", "This behavior doesn't go all the way, but we're better off than
before", or even to just take a week off and come back to it later.

So if you're feeling burned out on submitting patches, maybe that helps.

Of course, there are also just plain nitpicks. We try to keep them to a
minimum, but they do creep in. :)

-Peff

Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

From: Jay Soffian <redacted>

On Fri, Feb 13, 2009 at 6:32 PM, Thomas Rast [off-list ref] wrote:
Jay Soffian wrote:
quoted
-                     if (/^(Signed-off-by|Cc): (.*)$/i) {
-                             next if ($suppress_cc{'sob'});
[...]
quoted
+             if (/^(Signed-off-by|Cc): (.*)$/i) {
+                     next if ($suppress_cc{'sob'});
Doesn't this actually look like a long-standing send-email bug?  Since
6564828 (git-send-email: Generalize auto-cc recipient mechanism.,
2007-12-25) they should go in separate categories, but the above lines
were just translated from the old $signed_off_cc setting.  It seems
they should distinguish between SOB and Cc.
This is fixed by the last patch in this series. While I was working on
it I noticed two other minor issues, which is the first two patches.

This is meant to go on top of
http://thread.gmane.org/gmane.comp.version-control.git/109783

Jay Soffian (3):
  send-email: correct logic error with --suppress-cc=cc
  send-email: don't call unquote_rfc2047 unnecessarily
  send-email: --suppress-cc improvements

 Documentation/git-send-email.txt |   14 ++++++----
 git-send-email.perl              |   50 ++++++++++++++++++++++----------------
 t/t9001-send-email.sh            |   38 +++++++++++++++++++++++++++-
 3 files changed, 73 insertions(+), 29 deletions(-)

[PATCH 2/3] send-email: don't call unquote_rfc2047 unnecessarily

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

From: Jay Soffian <redacted>

If --suppress-cc=author then there is no need to unquote the From:
address.

Signed-off-by: Jay Soffian <redacted>
---
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index a2e0b94..2a3e3e8 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -958,8 +958,8 @@ foreach my $t (@files) {
 				$subject = $1;
 			}
 			elsif (/^From:\s+(.*)$/) {
+				next if $suppress_cc{'author'};
 				($author, $author_encoding) = unquote_rfc2047($1);
-				next if ($suppress_cc{'author'});
 				printf("(mbox) Adding cc: %s from line '%s'\n",
 					$1, $_) unless $quiet;
 				push @cc, $1;
-- 
1.6.2.rc0.235.g1319

[PATCH 1/3] send-email: correct logic error with --suppress-cc=cc

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

From: Jay Soffian <redacted>

--suppress-cc=cc is supposed to suppress harvesting addresses from any
Cc: lines. However, in the case where the Cc: line contained the sender,
it would only suppress if --suppress-cc=self.

Signed-off-by: Jay Soffian <redacted>
---
 git-send-email.perl |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index cde294c..a2e0b94 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -965,12 +965,10 @@ foreach my $t (@files) {
 				push @cc, $1;
 			}
 			elsif (/^Cc:\s+(.*)$/) {
+				next if $suppress_cc{'cc'};
 				foreach my $addr (parse_address_line($1)) {
-					if (unquote_rfc2047($addr) eq $sender) {
-						next if ($suppress_cc{'self'});
-					} else {
-						next if ($suppress_cc{'cc'});
-					}
+					next if $suppress_cc{'self'} and
+						unquote_rfc2047($addr) eq $sender;
 					printf("(mbox) Adding cc: %s from line '%s'\n",
 						$addr, $_) unless $quiet;
 					push @cc, $addr;
-- 
1.6.2.rc0.235.g1319

[PATCH 3/3] send-email: --suppress-cc improvements

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

From: Jay Soffian <redacted>

Commit 656482830ddc4a4e2af132fabb118a25190439c2 added the --suppress-cc
option. However, it made --suppress-cc=sob suppress both SOB lines and
body Cc lines (but not header Cc lines), which seems contrary to how
it is named.

After this commit, 'sob' suppresses only SOB lines and --suppress-cc
takes two additional values:

 * 'body' suppresses both SOB and body Cc lines (i.e. what 'sob'
    used to do).

 * 'bodycc' suppresses body Cc lines, but not header Cc lines.

For backwards compatibility, --no-signed-off-by-cc, acts like 'body'.

Also update the documentation and add a few tests.

Signed-off-by: Jay Soffian <redacted>
---

Okay, so --suppress=cc=sob now acts differently. I can hear the
backwards compatibility objection. I disagree. It will only effect a
user who does --suppress-cc=sob AND sends out a patch that has Cc: in
the message body (commit message). And If I'm not the first user to have
sent such a message earlier today, I'll bet I'm not much more than the
10th. 

OTOH, --suppress-cc=sob was obviously misnamed in the first place, and I
was confused by having send-email pick up the Cc in the body.

So I think it's worth the small risk of changing what --suppress-cc=sob
does. And as a benefit, we now offer two new values for --suppress-cc,
and for really-old-timers that are probably using --no-signed-of-by-cc,
that says the same.

j.

 Documentation/git-send-email.txt |   14 +++++++-----
 git-send-email.perl              |   40 +++++++++++++++++++++++--------------
 t/t9001-send-email.sh            |   38 ++++++++++++++++++++++++++++++++++-
 3 files changed, 69 insertions(+), 23 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index ff4aeff..d6af035 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -166,12 +166,14 @@ Automating
 	Specify an additional category of recipients to suppress the
 	auto-cc of.  'self' will avoid including the sender, 'author' will
 	avoid including the patch author, 'cc' will avoid including anyone
-	mentioned in Cc lines in the patch, 'sob' will avoid including
-	anyone mentioned in Signed-off-by lines, and 'cccmd' will avoid
-	running the --cc-cmd.  'all' will suppress all auto cc values.
-	Default is the value of 'sendemail.suppresscc' configuration value;
-	if that is unspecified, default to 'self' if --suppress-from is
-	specified, as well as 'sob' if --no-signed-off-cc is specified.
+	mentioned in Cc lines in the patch header, 'ccbody' will avoid
+	including anyone mentioned in Cc lines in the patch body (commit
+	message), 'sob' will avoid including anyone mentioned in Signed-off-by
+	lines, and 'cccmd' will avoid running the --cc-cmd. 'body' is
+	equivalent to 'sob' + 'ccbody'. 'all' will suppress all auto cc
+	values.  Default is the value of 'sendemail.suppresscc' configuration
+	value; if that is unspecified, default to 'self' if --suppress-from is
+	specified, as well as 'body' if --no-signed-off-cc is specified.
 
 --[no-]suppress-from::
 	If this is set, do not add the From: address to the cc: list.
diff --git a/git-send-email.perl b/git-send-email.perl
index 2a3e3e8..2282287 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -68,7 +68,7 @@ git send-email [options] <file | directory | rev-list options >
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
     --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
-    --suppress-cc           <str>  * author, self, sob, cccmd, all.
+    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, all.
     --[no-]signed-off-by-cc        * Send to Cc: and Signed-off-by:
                                      addresses. Default on.
     --[no-]suppress-from           * Send to self. Default off.
@@ -319,21 +319,28 @@ my(%suppress_cc);
 if (@suppress_cc) {
 	foreach my $entry (@suppress_cc) {
 		die "Unknown --suppress-cc field: '$entry'\n"
-			unless $entry =~ /^(all|cccmd|cc|author|self|sob)$/;
+			unless $entry =~ /^(all|cccmd|cc|author|self|sob|body|bodycc)$/;
 		$suppress_cc{$entry} = 1;
 	}
 }
 
 if ($suppress_cc{'all'}) {
-	foreach my $entry (qw (ccmd cc author self sob)) {
+	foreach my $entry (qw (ccmd cc author self sob body bodycc)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'all'};
 }
 
+if ($suppress_cc{'sob'} && $suppress_cc{'bodycc'}) {
+	$suppress_cc{'body'} = 1;
+}
+
 # If explicit old-style ones are specified, they trump --suppress-cc.
 $suppress_cc{'self'} = $suppress_from if defined $suppress_from;
-$suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;
+# For backwards compatibility, old-style --signed-off-by-cc suppresses
+# SOB and body Cc lines, whereas --supress-cc=sob suppresses just the SOB
+# line, but not the body Cc.
+$suppress_cc{'body'} = !$signed_off_by_cc if defined $signed_off_by_cc;
 
 # Debugging, print out the suppressions.
 if (0) {
@@ -1005,17 +1012,20 @@ foreach my $t (@files) {
 		}
 	}
 	# Now parse the message body
-	while(<F>) {
-		$message .=  $_;
-		if (/^(Signed-off-by|Cc): (.*)$/i) {
-			next if ($suppress_cc{'sob'});
-			chomp;
-			my $c = $2;
-			chomp $c;
-			next if ($c eq $sender and $suppress_cc{'self'});
-			push @cc, $c;
-			printf("(sob) Adding cc: %s from line '%s'\n",
-				$c, $_) unless $quiet;
+	unless ($suppress_cc{'body'}) {
+		while(<F>) {
+			$message .=  $_;
+			if (/^(Signed-off-by|Cc): (.*)$/i) {
+				chomp;
+				my ($what, $c) = ($1, $2);
+				chomp $c;
+				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
+				next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
+				next if ($c eq $sender and $suppress_cc{'self'});
+				push @cc, $c;
+				printf("(body) Adding cc: %s from line '%s'\n",
+					$c, $_) unless $quiet;
+			}
 		}
 	}
 	close F;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index da54835..d7766f9 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -32,11 +32,11 @@ clean_fake_sendmail() {
 }
 
 test_expect_success 'Extract patches' '
-    patches=`git format-patch --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1`
+    patches=`git format-patch -s --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1`
 '
 
 test_expect_success 'Send patches' '
-     git send-email --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors
+     git send-email --suppress-cc=sob --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors
 '
 
 cat >expected <<\EOF
@@ -74,6 +74,7 @@ EOF
 test_expect_success 'Show all headers' '
 	git send-email \
 		--dry-run \
+		--suppress-cc=sob \
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
 		--cc=cc@example.com \
@@ -195,6 +196,7 @@ test_expect_success 'sendemail.cc set' '
 	git config sendemail.cc cc@example.com &&
 	git send-email \
 		--dry-run \
+		--suppress-cc=sob \
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
 		--smtp-server relay.example.com \
@@ -230,6 +232,38 @@ test_expect_success 'sendemail.cc unset' '
 	git config --unset sendemail.cc &&
 	git send-email \
 		--dry-run \
+		--suppress-cc=sob \
+		--from="Example <from@example.com>" \
+		--to=to@example.com \
+		--smtp-server relay.example.com \
+		$patches |
+	sed	-e "s/^\(Date:\).*/\1 DATE-STRING/" \
+		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
+		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
+		>actual-show-all-headers &&
+	test_cmp expected-show-all-headers actual-show-all-headers
+'
+
+cat >expected-show-all-headers <<\EOF
+0001-Second.patch
+Dry-OK. Log says:
+Server: relay.example.com
+MAIL FROM:<from@example.com>
+RCPT TO:<to@example.com>
+From: Example <from@example.com>
+To: to@example.com
+Subject: [PATCH 1/1] Second.
+Date: DATE-STRING
+Message-Id: MESSAGE-ID-STRING
+X-Mailer: X-MAILER-STRING
+
+Result: OK
+EOF
+
+test_expect_success '--suppress-cc=all' '
+	git send-email \
+		--dry-run \
+		--suppress-cc=all \
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
 		--smtp-server relay.example.com \
-- 
1.6.2.rc0.235.g1319

[PATCH 3/3 v2] send-email: --suppress-cc improvements

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

From: Jay Soffian <redacted>

Commit 656482830ddc4a4e2af132fabb118a25190439c2 added the --suppress-cc
option. However, it made --suppress-cc=sob suppress both SOB lines and
body Cc lines (but not header Cc lines), which seems contrary to how
it is named.

After this commit, 'sob' suppresses only SOB lines and --suppress-cc
takes two additional values:

 * 'body' suppresses both SOB and body Cc lines (i.e. what 'sob'
    used to do).

 * 'bodycc' suppresses body Cc lines, but not header Cc lines.

For backwards compatibility, --no-signed-off-by-cc, acts like 'body'.

Also update the documentation and add a few tests.

Signed-off-by: Jay Soffian <redacted>
---
Sorry, please ignore the previous 3/3, it had an obvious breakage;
--suppress-cc=body was causing then entire message body not to be read
in at all.

 Documentation/git-send-email.txt |   14 ++++++++------
 git-send-email.perl              |   23 ++++++++++++++++-------
 t/t9001-send-email.sh            |   38 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 60 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index ff4aeff..d6af035 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -166,12 +166,14 @@ Automating
 	Specify an additional category of recipients to suppress the
 	auto-cc of.  'self' will avoid including the sender, 'author' will
 	avoid including the patch author, 'cc' will avoid including anyone
-	mentioned in Cc lines in the patch, 'sob' will avoid including
-	anyone mentioned in Signed-off-by lines, and 'cccmd' will avoid
-	running the --cc-cmd.  'all' will suppress all auto cc values.
-	Default is the value of 'sendemail.suppresscc' configuration value;
-	if that is unspecified, default to 'self' if --suppress-from is
-	specified, as well as 'sob' if --no-signed-off-cc is specified.
+	mentioned in Cc lines in the patch header, 'ccbody' will avoid
+	including anyone mentioned in Cc lines in the patch body (commit
+	message), 'sob' will avoid including anyone mentioned in Signed-off-by
+	lines, and 'cccmd' will avoid running the --cc-cmd. 'body' is
+	equivalent to 'sob' + 'ccbody'. 'all' will suppress all auto cc
+	values.  Default is the value of 'sendemail.suppresscc' configuration
+	value; if that is unspecified, default to 'self' if --suppress-from is
+	specified, as well as 'body' if --no-signed-off-cc is specified.
 
 --[no-]suppress-from::
 	If this is set, do not add the From: address to the cc: list.
diff --git a/git-send-email.perl b/git-send-email.perl
index 2a3e3e8..23a55e2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -68,7 +68,7 @@ git send-email [options] <file | directory | rev-list options >
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
     --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
-    --suppress-cc           <str>  * author, self, sob, cccmd, all.
+    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, all.
     --[no-]signed-off-by-cc        * Send to Cc: and Signed-off-by:
                                      addresses. Default on.
     --[no-]suppress-from           * Send to self. Default off.
@@ -319,21 +319,28 @@ my(%suppress_cc);
 if (@suppress_cc) {
 	foreach my $entry (@suppress_cc) {
 		die "Unknown --suppress-cc field: '$entry'\n"
-			unless $entry =~ /^(all|cccmd|cc|author|self|sob)$/;
+			unless $entry =~ /^(all|cccmd|cc|author|self|sob|body|bodycc)$/;
 		$suppress_cc{$entry} = 1;
 	}
 }
 
 if ($suppress_cc{'all'}) {
-	foreach my $entry (qw (ccmd cc author self sob)) {
+	foreach my $entry (qw (ccmd cc author self sob body bodycc)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'all'};
 }
 
+if ($suppress_cc{'sob'} && $suppress_cc{'bodycc'}) {
+	$suppress_cc{'body'} = 1;
+}
+
 # If explicit old-style ones are specified, they trump --suppress-cc.
 $suppress_cc{'self'} = $suppress_from if defined $suppress_from;
-$suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;
+# For backwards compatibility, old-style --signed-off-by-cc suppresses
+# SOB and body Cc lines, whereas --supress-cc=sob suppresses just the SOB
+# line, but not the body Cc.
+$suppress_cc{'body'} = !$signed_off_by_cc if defined $signed_off_by_cc;
 
 # Debugging, print out the suppressions.
 if (0) {
@@ -1007,14 +1014,16 @@ foreach my $t (@files) {
 	# Now parse the message body
 	while(<F>) {
 		$message .=  $_;
+		next if $suppress_cc{'body'};
 		if (/^(Signed-off-by|Cc): (.*)$/i) {
-			next if ($suppress_cc{'sob'});
 			chomp;
-			my $c = $2;
+			my ($what, $c) = ($1, $2);
 			chomp $c;
+			next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
+			next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
 			next if ($c eq $sender and $suppress_cc{'self'});
 			push @cc, $c;
-			printf("(sob) Adding cc: %s from line '%s'\n",
+			printf("(body) Adding cc: %s from line '%s'\n",
 				$c, $_) unless $quiet;
 		}
 	}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index da54835..d7766f9 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -32,11 +32,11 @@ clean_fake_sendmail() {
 }
 
 test_expect_success 'Extract patches' '
-    patches=`git format-patch --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1`
+    patches=`git format-patch -s --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1`
 '
 
 test_expect_success 'Send patches' '
-     git send-email --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors
+     git send-email --suppress-cc=sob --from="Example <nobody@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors
 '
 
 cat >expected <<\EOF
@@ -74,6 +74,7 @@ EOF
 test_expect_success 'Show all headers' '
 	git send-email \
 		--dry-run \
+		--suppress-cc=sob \
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
 		--cc=cc@example.com \
@@ -195,6 +196,7 @@ test_expect_success 'sendemail.cc set' '
 	git config sendemail.cc cc@example.com &&
 	git send-email \
 		--dry-run \
+		--suppress-cc=sob \
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
 		--smtp-server relay.example.com \
@@ -230,6 +232,38 @@ test_expect_success 'sendemail.cc unset' '
 	git config --unset sendemail.cc &&
 	git send-email \
 		--dry-run \
+		--suppress-cc=sob \
+		--from="Example <from@example.com>" \
+		--to=to@example.com \
+		--smtp-server relay.example.com \
+		$patches |
+	sed	-e "s/^\(Date:\).*/\1 DATE-STRING/" \
+		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
+		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
+		>actual-show-all-headers &&
+	test_cmp expected-show-all-headers actual-show-all-headers
+'
+
+cat >expected-show-all-headers <<\EOF
+0001-Second.patch
+Dry-OK. Log says:
+Server: relay.example.com
+MAIL FROM:<from@example.com>
+RCPT TO:<to@example.com>
+From: Example <from@example.com>
+To: to@example.com
+Subject: [PATCH 1/1] Second.
+Date: DATE-STRING
+Message-Id: MESSAGE-ID-STRING
+X-Mailer: X-MAILER-STRING
+
+Result: OK
+EOF
+
+test_expect_success '--suppress-cc=all' '
+	git send-email \
+		--dry-run \
+		--suppress-cc=all \
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
 		--smtp-server relay.example.com \
-- 
1.6.2.rc0.238.g0c1fe

Re: [PATCH 2/3] send-email: don't call unquote_rfc2047 unnecessarily

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

On Fri, Feb 13, 2009 at 10:51 PM, Jay Soffian [off-list ref] wrote:
quoted hunk
From: Jay Soffian <redacted>

If --suppress-cc=author then there is no need to unquote the From:
address.

Signed-off-by: Jay Soffian <redacted>
---
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index a2e0b94..2a3e3e8 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -958,8 +958,8 @@ foreach my $t (@files) {
                               $subject = $1;
                       }
                       elsif (/^From:\s+(.*)$/) {
+                               next if $suppress_cc{'author'};
                               ($author, $author_encoding) = unquote_rfc2047($1);
-                               next if ($suppress_cc{'author'});
                               printf("(mbox) Adding cc: %s from line '%s'\n",
                                       $1, $_) unless $quiet;
                               push @cc, $1;
Crap. Ignore this one from the series. $author is used later and we
have to parse it regardless. This is brittle code, but I should've
realize it.

j.

[PATCH 1/3 v2] send-email: correct logic error with --suppress-cc=cc

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:10

--suppress-cc=cc is supposed to suppress harvesting addresses from any
Cc: lines. However, in the case where the Cc: line contained the sender,
it would only suppress if --suppress-cc=self.

Signed-off-by: Jay Soffian <redacted>
---
And here's a re-roll of this one. Sheesh, that logic was too subtle for
my brain. I think it's a lot more obvious what is going on after this
patch.

I apologize that this thread is getting so dense. Here it is on gmane
which may help:

  http://thread.gmane.org/gmane.comp.version-control.git/109783

j.

 git-send-email.perl |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index cde294c..cef32da 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -959,18 +959,17 @@ foreach my $t (@files) {
 			}
 			elsif (/^From:\s+(.*)$/) {
 				($author, $author_encoding) = unquote_rfc2047($1);
-				next if ($suppress_cc{'author'});
+				next if $suppress_cc{'author'};
+				next if $suppress_cc{'self'} and $author eq $sender;
 				printf("(mbox) Adding cc: %s from line '%s'\n",
 					$1, $_) unless $quiet;
 				push @cc, $1;
 			}
 			elsif (/^Cc:\s+(.*)$/) {
+				next if $suppress_cc{'cc'};
 				foreach my $addr (parse_address_line($1)) {
-					if (unquote_rfc2047($addr) eq $sender) {
-						next if ($suppress_cc{'self'});
-					} else {
-						next if ($suppress_cc{'cc'});
-					}
+					next if $suppress_cc{'self'} and
+						unquote_rfc2047($addr) eq $sender;
 					printf("(mbox) Adding cc: %s from line '%s'\n",
 						$addr, $_) unless $quiet;
 					push @cc, $addr;
@@ -1038,7 +1037,7 @@ foreach my $t (@files) {
 			or die "(cc-cmd) failed to close pipe to '$cc_cmd'";
 	}
 
-	if (defined $author) {
+	if (defined $author and $author ne $sender) {
 		$message = "From: $author\n\n$message";
 		if (defined $author_encoding) {
 			if ($has_content_type) {
-- 
1.6.2.rc0.238.g0c1fe

Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message

From: Thomas Rast <hidden>
Date: 2016-06-15 22:46:10

Jay Soffian wrote:
From: Jay Soffian <redacted>

On Fri, Feb 13, 2009 at 6:32 PM, Thomas Rast [off-list ref] wrote:
quoted
Jay Soffian wrote:
quoted
-                     if (/^(Signed-off-by|Cc): (.*)$/i) {
-                             next if ($suppress_cc{'sob'});
[...]
quoted
+             if (/^(Signed-off-by|Cc): (.*)$/i) {
+                     next if ($suppress_cc{'sob'});
Doesn't this actually look like a long-standing send-email bug?  Since
6564828 (git-send-email: Generalize auto-cc recipient mechanism.,
2007-12-25) they should go in separate categories, but the above lines
were just translated from the old $signed_off_cc setting.  It seems
they should distinguish between SOB and Cc.
This is fixed by the last patch in this series. While I was working on
it I noticed two other minor issues, which is the first two patches.
Thanks for the fix, and sorry if this was the comment that inspired
your venting earlier.  I should've tried a patch myself, but it was
already past midnight.  I'll send a slightly improved version of 3/3
shortly.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help