{"thread":{"id":"17780","subject":"[PATCH] send-email: handle multiple Cc addresses when reading mbox message","startedAt":"2009-02-13T23:05:13Z","lastAt":"2009-02-14T16:57:14Z","messageCount":20,"participants":["Jay Soffian","Thomas Rast","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"104551","messageId":"1234566313-36478-1-git-send-email-jaysoffian@gmail.com","threadId":"17780","inReplyTo":null,"subject":"[PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T23:05:13Z","receivedAt":"2009-02-13T23:05:13Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"From: Jay Soffian \n\nWhen git format-patch is given multiple --cc arguments, it generates a\nCc header that looks like:\n\n Cc: first@example.com,\n second@example.com,\n third@example.com\n\nBefore this commit, send-email was unable to handle such a message.\nFirst, it didn't know how to handle a header split across multiple\nlines. Second, it couldn't handle multiple comma-separated addresses,\neven if they were provided all on a single Cc: line.\n\nThis patch teaches it to unfold header lines by pre-processing the\nheader before extracting any of its fields. It also teaches it to split\nthe Cc line using Mail::Address if available, otherwise it just splits\non a simple \\s*,\\s* regular expression.\n\nAlso, conditionally use Mail::Address to handle the response to the \"Who\nshould the emails be sent to?\" prompt.\n\nSigned-off-by: Jay Soffian \n---\nThis message which you are reading was generated with:\n$ git format-patch --cc=\"Ryan Anderson \" \\\n --cc=gitster@pobox.com -s -1\n\nAnd sent with:\n$ git send-email --from 'Jay Soffian ' \\\n --to git@vger.kernel.org --cc 'Jay Soffian '\n\nBefore this patch, send-email would not have sent it properly. :-)\n\nj.\n\n git-send-email.perl | 150 +++++++++++++++++++++++++++---------------------\n t/t9001-send-email.sh | 22 +++++--\n 2 files changed, 99 insertions(+), 73 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 77ca8fe..cde294c 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -126,6 +126,7 @@ sub format_2822_time {\n }\n \n my $have_email_valid = eval { require Email::Valid; 1 };\n+my $have_mail_address = eval { require Mail::Address; 1 };\n my $smtp;\n my $auth;\n \n@@ -360,6 +361,14 @@ foreach my $entry (@bcclist) {\n \tdie \"Comma in --bcclist entry: $entry'\\n\" unless $entry !~ m/,/;\n }\n \n+sub parse_address_line {\n+\tif ($have_mail_address) {\n+\t\treturn map { $_->format } Mail::Address->parse($_[0]);\n+\t} else {\n+\t\treturn split_addrs($_[0]);\n+\t}\n+}\n+\n sub split_addrs {\n \treturn quotewords('\\s*,\\s*', 1, @_);\n }\n@@ -593,7 +602,7 @@ if (!@to) {\n \t}\n \n \tmy $to = $_;\n-\tpush @to, split_addrs($to);\n+\tpush @to, parse_address_line($to);\n \t$prompting++;\n }\n \n@@ -920,88 +929,97 @@ foreach my $t (@files) {\n \t@cc = @initial_cc;\n \t@xh = ();\n \tmy $input_format = undef;\n-\tmy $header_done = 0;\n+\tmy @header = ();\n \t$message = \"\";\n+\t# First unfold multiline header fields\n \twhile() {\n-\t\tif (!$header_done) {\n-\t\t\tif (/^From /) {\n-\t\t\t\t$input_format = 'mbox';\n-\t\t\t\tnext;\n+\t\tlast if /^\\s*$/;\n+\t\tif (/^\\s+\\S/ and @header) {\n+\t\t\tchomp($header[$#header]);\n+\t\t\ts/^\\s+/ /;\n+\t\t\t$header[$#header] .= $_;\n+\t } else {\n+\t\t\tpush(@header, $_);\n+\t\t}\n+\t}\n+\t# Now parse the header\n+\tforeach(@header) {\n+\t\tif (/^From /) {\n+\t\t\t$input_format = 'mbox';\n+\t\t\tnext;\n+\t\t}\n+\t\tchomp;\n+\t\tif (!defined $input_format && /^[-A-Za-z]+:\\s/) {\n+\t\t\t$input_format = 'mbox';\n+\t\t}\n+\n+\t\tif (defined $input_format && $input_format eq 'mbox') {\n+\t\t\tif (/^Subject:\\s+(.*)$/) {\n+\t\t\t\t$subject = $1;\n \t\t\t}\n-\t\t\tchomp;\n-\t\t\tif (!defined $input_format && /^[-A-Za-z]+:\\s/) {\n-\t\t\t\t$input_format = 'mbox';\n+\t\t\telsif (/^From:\\s+(.*)$/) {\n+\t\t\t\t($author, $author_encoding) = unquote_rfc2047($1);\n+\t\t\t\tnext if ($suppress_cc{'author'});\n+\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n+\t\t\t\t\t$1, $_) unless $quiet;\n+\t\t\t\tpush @cc, $1;\n \t\t\t}\n-\n-\t\t\tif (defined $input_format && $input_format eq 'mbox') {\n-\t\t\t\tif (/^Subject:\\s+(.*)$/) {\n-\t\t\t\t\t$subject = $1;\n-\n-\t\t\t\t} elsif (/^(Cc|From):\\s+(.*)$/) {\n-\t\t\t\t\tif (unquote_rfc2047($2) eq $sender) {\n+\t\t\telsif (/^Cc:\\s+(.*)$/) {\n+\t\t\t\tforeach my $addr (parse_address_line($1)) {\n+\t\t\t\t\tif (unquote_rfc2047($addr) eq $sender) {\n \t\t\t\t\t\tnext if ($suppress_cc{'self'});\n-\t\t\t\t\t}\n-\t\t\t\t\telsif ($1 eq 'From') {\n-\t\t\t\t\t\t($author, $author_encoding)\n-\t\t\t\t\t\t = unquote_rfc2047($2);\n-\t\t\t\t\t\tnext if ($suppress_cc{'author'});\n \t\t\t\t\t} else {\n \t\t\t\t\t\tnext if ($suppress_cc{'cc'});\n \t\t\t\t\t}\n \t\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t\t\t$2, $_) unless $quiet;\n-\t\t\t\t\tpush @cc, $2;\n+\t\t\t\t\t\t$addr, $_) unless $quiet;\n+\t\t\t\t\tpush @cc, $addr;\n \t\t\t\t}\n-\t\t\t\telsif (/^Content-type:/i) {\n-\t\t\t\t\t$has_content_type = 1;\n-\t\t\t\t\tif (/charset=\"?([^ \"]+)/) {\n-\t\t\t\t\t\t$body_encoding = $1;\n-\t\t\t\t\t}\n-\t\t\t\t\tpush @xh, $_;\n-\t\t\t\t}\n-\t\t\t\telsif (/^Message-Id: (.*)/i) {\n-\t\t\t\t\t$message_id = $1;\n-\t\t\t\t}\n-\t\t\t\telsif (!/^Date:\\s/ && /^[-A-Za-z]+:\\s+\\S/) {\n-\t\t\t\t\tpush @xh, $_;\n-\t\t\t\t}\n-\n-\t\t\t} else {\n-\t\t\t\t# In the traditional\n-\t\t\t\t# \"send lots of email\" format,\n-\t\t\t\t# line 1 = cc\n-\t\t\t\t# line 2 = subject\n-\t\t\t\t# So let's support that, too.\n-\t\t\t\t$input_format = 'lots';\n-\t\t\t\tif (@cc == 0 && !$suppress_cc{'cc'}) {\n-\t\t\t\t\tprintf(\"(non-mbox) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t\t\t$_, $_) unless $quiet;\n-\n-\t\t\t\t\tpush @cc, $_;\n-\n-\t\t\t\t} elsif (!defined $subject) {\n-\t\t\t\t\t$subject = $_;\n+\t\t\t}\n+\t\t\telsif (/^Content-type:/i) {\n+\t\t\t\t$has_content_type = 1;\n+\t\t\t\tif (/charset=\"?([^ \"]+)/) {\n+\t\t\t\t\t$body_encoding = $1;\n \t\t\t\t}\n+\t\t\t\tpush @xh, $_;\n \t\t\t}\n-\n-\t\t\t# A whitespace line will terminate the headers\n-\t\t\tif (m/^\\s*$/) {\n-\t\t\t\t$header_done = 1;\n+\t\t\telsif (/^Message-Id: (.*)/i) {\n+\t\t\t\t$message_id = $1;\n \t\t\t}\n+\t\t\telsif (!/^Date:\\s/ && /^[-A-Za-z]+:\\s+\\S/) {\n+\t\t\t\tpush @xh, $_;\n+\t\t\t}\n+\n \t\t} else {\n-\t\t\t$message .= $_;\n-\t\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n-\t\t\t\tnext if ($suppress_cc{'sob'});\n-\t\t\t\tchomp;\n-\t\t\t\tmy $c = $2;\n-\t\t\t\tchomp $c;\n-\t\t\t\tnext if ($c eq $sender and $suppress_cc{'self'});\n-\t\t\t\tpush @cc, $c;\n-\t\t\t\tprintf(\"(sob) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t\t$c, $_) unless $quiet;\n+\t\t\t# In the traditional\n+\t\t\t# \"send lots of email\" format,\n+\t\t\t# line 1 = cc\n+\t\t\t# line 2 = subject\n+\t\t\t# So let's support that, too.\n+\t\t\t$input_format = 'lots';\n+\t\t\tif (@cc == 0 && !$suppress_cc{'cc'}) {\n+\t\t\t\tprintf(\"(non-mbox) Adding cc: %s from line '%s'\\n\",\n+\t\t\t\t\t$_, $_) unless $quiet;\n+\t\t\t\tpush @cc, $_;\n+\t\t\t} elsif (!defined $subject) {\n+\t\t\t\t$subject = $_;\n \t\t\t}\n \t\t}\n \t}\n+\t# Now parse the message body\n+\twhile() {\n+\t\t$message .= $_;\n+\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n+\t\t\tnext if ($suppress_cc{'sob'});\n+\t\t\tchomp;\n+\t\t\tmy $c = $2;\n+\t\t\tchomp $c;\n+\t\t\tnext if ($c eq $sender and $suppress_cc{'self'});\n+\t\t\tpush @cc, $c;\n+\t\t\tprintf(\"(sob) Adding cc: %s from line '%s'\\n\",\n+\t\t\t\t$c, $_) unless $quiet;\n+\t\t}\n+\t}\n \tclose F;\n \n \tif (defined $cc_cmd && !$suppress_cc{'cccmd'}) {\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex cb3d183..da54835 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -32,7 +32,7 @@ clean_fake_sendmail() {\n }\n \n test_expect_success 'Extract patches' '\n- patches=`git format-patch -n HEAD^1`\n+ patches=`git format-patch --cc=\"One \" --cc=two@example.com -n HEAD^1`\n '\n \n test_expect_success 'Send patches' '\n@@ -42,6 +42,8 @@ test_expect_success 'Send patches' '\n cat >expected <<\\EOF\n !nobody@example.com!\n !author@example.com!\n+!one@example.com!\n+!two@example.com!\n EOF\n test_expect_success \\\n 'Verify commandline' \\\n@@ -50,13 +52,15 @@ test_expect_success \\\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n (mbox) Adding cc: A from line 'From: A '\n+(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n+(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n-RCPT TO:,,,\n+RCPT TO:,,,,,\n From: Example \n To: to@example.com\n-Cc: cc@example.com, A \n+Cc: cc@example.com, A , One , two@example.com\n Subject: [PATCH 1/1] Second.\n Date: DATE-STRING\n Message-Id: MESSAGE-ID-STRING\n@@ -170,13 +174,15 @@ test_expect_success 'second message is patch' '\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n (mbox) Adding cc: A from line 'From: A '\n+(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n+(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n-RCPT TO:,,\n+RCPT TO:,,,,\n From: Example \n To: to@example.com\n-Cc: cc@example.com, A \n+Cc: cc@example.com, A , One , two@example.com\n Subject: [PATCH 1/1] Second.\n Date: DATE-STRING\n Message-Id: MESSAGE-ID-STRING\n@@ -203,13 +209,15 @@ test_expect_success 'sendemail.cc set' '\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n (mbox) Adding cc: A from line 'From: A '\n+(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n+(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n-RCPT TO:,\n+RCPT TO:,,,\n From: Example \n To: to@example.com\n-Cc: A \n+Cc: A , One , two@example.com\n Subject: [PATCH 1/1] Second.\n Date: DATE-STRING\n Message-Id: MESSAGE-ID-STRING\n-- \n1.6.2.rc0.67.g77afc\n"},{"id":"104553","messageId":"200902140032.19922.trast@student.ethz.ch","threadId":"17780","inReplyTo":"1234566313-36478-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-13T23:32:15Z","receivedAt":"2009-02-13T23:32:15Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jay Soffian wrote:\n> -\t\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n> -\t\t\t\tnext if ($suppress_cc{'sob'});\n[...]\n> +\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n> +\t\t\tnext if ($suppress_cc{'sob'});\n\nDoesn't this actually look like a long-standing send-email bug? Since\n6564828 (git-send-email: Generalize auto-cc recipient mechanism.,\n2007-12-25) they should go in separate categories, but the above lines\nwere just translated from the old $signed_off_cc setting. It seems\nthey should distinguish between SOB and Cc.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"104554","messageId":"76718490902131539o15439f12l7d4a8113f482c6ee@mail.gmail.com","threadId":"17780","inReplyTo":"200902140032.19922.trast@student.ethz.ch","subject":"Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T23:39:14Z","receivedAt":"2009-02-13T23:39:14Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 6:32 PM, Thomas Rast wrote:\n> Jay Soffian wrote:\n>> - if (/^(Signed-off-by|Cc): (.*)$/i) {\n>> - next if ($suppress_cc{'sob'});\n> [...]\n>> + if (/^(Signed-off-by|Cc): (.*)$/i) {\n>> + next if ($suppress_cc{'sob'});\n>\n> Doesn't this actually look like a long-standing send-email bug? Since\n> 6564828 (git-send-email: Generalize auto-cc recipient mechanism.,\n> 2007-12-25) they should go in separate categories, but the above lines\n> were just translated from the old $signed_off_cc setting. It seems\n> they should distinguish between SOB and Cc.\n\nI think it would be nice if send-email had different settings for\nextracting Cc lines and extracting SOB lines from the message body,\nbut I don't call that a bug, but rather an enhancement request.\n\nAnd yes, I noticed it when I went to send this patch because\noriginally I had \"Cc: ...\" in the commit message on column 0, which\nsend-email annoyingly picked up. I just changed my commit message to\nmove the \"Cc:\" to column 1.\n\nNonetheless, it is independent of this patch.\n\nj.\n"},{"id":"104555","messageId":"76718490902131544kd4c2d83w64c062d8705ef61d@mail.gmail.com","threadId":"17780","inReplyTo":"76718490902131539o15439f12l7d4a8113f482c6ee@mail.gmail.com","subject":"Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T23:44:00Z","receivedAt":"2009-02-13T23:44:00Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 6:39 PM, Jay Soffian wrote:\n> I think it would be nice if send-email had different settings for\n> extracting Cc lines and extracting SOB lines from the message body,\n> but I don't call that a bug, but rather an enhancement request.\n\nOh, I see it does now that I looked at that commit. Hmphf. I'll fix\nthat on-top of the patch I just sent.\n\nj.\n"},{"id":"104559","messageId":"20090214003113.GB7769@coredump.intra.peff.net","threadId":"17780","inReplyTo":"1234566313-36478-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T00:31:14Z","receivedAt":"2009-02-14T00:31:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 13, 2009 at 06:05:13PM -0500, Jay Soffian wrote:\n\n> This patch teaches it to unfold header lines by pre-processing the\n> header before extracting any of its fields. It also teaches it to split\n> the Cc line using Mail::Address if available, otherwise it just splits\n> on a simple \\s*,\\s* regular expression.\n\nUgh. This is not in any way a comment on your patch, but it is really\npainful to see time and again send-email bugs like this that could be\nsolved by using the wealth of existing perl modules.\n\nI think send-email is stuck somewhere between \"here is the bare minimum\nto send some patches, and we don't want to require any fancy\ndependencies\" and \"a full-fledged replacement for my MUA when dealing\nwith patches\".\n\nThe approach of \"use this module if we have it, otherwise do something\nquick and dirty\" might be the best approach. But I do wonder if the\n\"quick and dirty\" code path will end up buggy due to lack of use, and/or\nbe surprising for people who expect send-email to be more thorough.\n\nThe other alternatives are to keep fixing bugs and improving\nincrementally until it converges on being an actual MUA, or to simply\nstart a rewrite based on reasonable CPAN modules.\n\nI don't know what the right solution is. I am certainly not volunteering\nto rewrite; I am very happy just using my actual MUA to send patches. :)\n\n-Peff\n"},{"id":"104561","messageId":"76718490902131642y62b81bfat129cfbeda7957ff8@mail.gmail.com","threadId":"17780","inReplyTo":"20090214003113.GB7769@coredump.intra.peff.net","subject":"Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T00:42:16Z","receivedAt":"2009-02-14T00:42:16Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 7:31 PM, Jeff King wrote:\n> I don't know what the right solution is. I am certainly not volunteering\n> to rewrite; I am very happy just using my actual MUA to send patches. :)\n\nIt is quite ugly, and I have thought about rewriting it.\n\nBut to be quite honest, I have become very discouraged over the last\ntwo weeks about submitting patches. What seems at first to be a simple\nchange has required multiple revisions over frankly what feels to me\nlike nit-picking.\n\nIt goes like this:\n\nMe: Notice wart\nMe: Cut wart off (i.e. submit patch)\nReviewer: Sorry, you didn't cut that wart off to my standards, unless\nyou can do it better, I'd rather have the wart.\n\nI dunno, maybe I'm too sensitive and don't have the fortitude for\ncontributing to git.\n\nOr maybe I'm just venting on a Friday.\n\nj.\n"},{"id":"104581","messageId":"20090214033749.GA24545@coredump.intra.peff.net","threadId":"17780","inReplyTo":"76718490902131642y62b81bfat129cfbeda7957ff8@mail.gmail.com","subject":"Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T03:37:49Z","receivedAt":"2009-02-14T03:37:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 13, 2009 at 07:42:16PM -0500, Jay Soffian wrote:\n\n> On Fri, Feb 13, 2009 at 7:31 PM, Jeff King wrote:\n> > I don't know what the right solution is. I am certainly not volunteering\n> > to rewrite; I am very happy just using my actual MUA to send patches. :)\n> \n> It is quite ugly, and I have thought about rewriting it.\n> \n> But to be quite honest, I have become very discouraged over the last\n> two weeks about submitting patches. What seems at first to be a simple\n> change has required multiple revisions over frankly what feels to me\n> like nit-picking.\n\nI hope it wasn't my message in this thread that sent you over the edge.\nI really did mean the \"this isn't a comment on your patch\" but was just\nmusing out loud over what the right direction for send-email is in\ngeneral.\n\n> Me: Notice wart\n> Me: Cut wart off (i.e. submit patch)\n> Reviewer: Sorry, you didn't cut that wart off to my standards, unless\n> you can do it better, I'd rather have the wart.\n> \n> I dunno, maybe I'm too sensitive and don't have the fortitude for\n> contributing to git.\n> \n> Or maybe I'm just venting on a Friday.\n\nI hope just venting. I think the warts you've been cutting off have been\nimproving git.\n\nOne of the things I have seen happening with your patches is something\nlike:\n\n Jay: Here's a patch.\n Reviewer: Good, but it should also do X to be complete.\n or\n Good, but there is one corner case where the correct\n behavior is unclear [queue endless discussion, production of\n several versions of the patch doing different behavior, or\n realization that some of the behavior would be an order of\n magnitude more work to implement]\n\nThose sorts of reviews are good for helping git in the long run, and I\nthink reviewers are often thinking in terms of git's overall\nprogression. But I think it is also OK sometimes as a patch submitter to\nsay \"This series scratches my itch, and I am done for now.\" And maybe\nsomebody else picks it up and builds on it. Or maybe you do, but weeks\nor months later. Or maybe nobody does, because it turns out that nobody\nreally cares about that corner case in practice.\n\nWhich is really just \"perfect is the enemy of the good\" in another form,\nand recognizing that git's development is incremental.\n\nAnd all of that isn't to say there aren't plenty of patches that should\nbe rejected for incompleteness or lack of handling corner cases. Many\ntimes an incomplete solution is worse than no solution at all. So there\nis some judgement required about whether the changes are a net positive\nand a net negative.\n\nBut I think it is up to the submitter to decide how far he wants to take\na given topic (and at what rate), and to say \"OK, I'll go implement\nthat\", \"This behavior doesn't go all the way, but we're better off than\nbefore\", or even to just take a week off and come back to it later.\n\nSo if you're feeling burned out on submitting patches, maybe that helps.\n\nOf course, there are also just plain nitpicks. We try to keep them to a\nminimum, but they do creep in. :)\n\n-Peff\n"},{"id":"104583","messageId":"1234583491-61260-1-git-send-email-jaysoffian@gmail.com","threadId":"17780","inReplyTo":"200902140032.19922.trast@student.ethz.ch","subject":"Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T03:51:28Z","receivedAt":"2009-02-14T03:51:28Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"From: Jay Soffian \n\nOn Fri, Feb 13, 2009 at 6:32 PM, Thomas Rast wrote:\n> Jay Soffian wrote:\n>> - if (/^(Signed-off-by|Cc): (.*)$/i) {\n>> - next if ($suppress_cc{'sob'});\n> [...]\n>> + if (/^(Signed-off-by|Cc): (.*)$/i) {\n>> + next if ($suppress_cc{'sob'});\n>\n> Doesn't this actually look like a long-standing send-email bug? Since\n> 6564828 (git-send-email: Generalize auto-cc recipient mechanism.,\n> 2007-12-25) they should go in separate categories, but the above lines\n> were just translated from the old $signed_off_cc setting. It seems\n> they should distinguish between SOB and Cc.\n\nThis is fixed by the last patch in this series. While I was working on\nit I noticed two other minor issues, which is the first two patches.\n\nThis is meant to go on top of\nhttp://thread.gmane.org/gmane.comp.version-control.git/109783\n\nJay Soffian (3):\n send-email: correct logic error with --suppress-cc=cc\n send-email: don't call unquote_rfc2047 unnecessarily\n send-email: --suppress-cc improvements\n\n Documentation/git-send-email.txt | 14 ++++++----\n git-send-email.perl | 50 ++++++++++++++++++++++----------------\n t/t9001-send-email.sh | 38 +++++++++++++++++++++++++++-\n 3 files changed, 73 insertions(+), 29 deletions(-)\n"},{"id":"104585","messageId":"1234583491-61260-2-git-send-email-jaysoffian@gmail.com","threadId":"17780","inReplyTo":"1234583491-61260-1-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 1/3] send-email: correct logic error with --suppress-cc=cc","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T03:51:29Z","receivedAt":"2009-02-14T03:51:29Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"From: Jay Soffian \n\n--suppress-cc=cc is supposed to suppress harvesting addresses from any\nCc: lines. However, in the case where the Cc: line contained the sender,\nit would only suppress if --suppress-cc=self.\n\nSigned-off-by: Jay Soffian \n---\n git-send-email.perl | 8 +++-----\n 1 files changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex cde294c..a2e0b94 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -965,12 +965,10 @@ foreach my $t (@files) {\n \t\t\t\tpush @cc, $1;\n \t\t\t}\n \t\t\telsif (/^Cc:\\s+(.*)$/) {\n+\t\t\t\tnext if $suppress_cc{'cc'};\n \t\t\t\tforeach my $addr (parse_address_line($1)) {\n-\t\t\t\t\tif (unquote_rfc2047($addr) eq $sender) {\n-\t\t\t\t\t\tnext if ($suppress_cc{'self'});\n-\t\t\t\t\t} else {\n-\t\t\t\t\t\tnext if ($suppress_cc{'cc'});\n-\t\t\t\t\t}\n+\t\t\t\t\tnext if $suppress_cc{'self'} and\n+\t\t\t\t\t\tunquote_rfc2047($addr) eq $sender;\n \t\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t\t\t$addr, $_) unless $quiet;\n \t\t\t\t\tpush @cc, $addr;\n-- \n1.6.2.rc0.235.g1319\n"},{"id":"104584","messageId":"1234583491-61260-3-git-send-email-jaysoffian@gmail.com","threadId":"17780","inReplyTo":"1234583491-61260-2-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 2/3] send-email: don't call unquote_rfc2047 unnecessarily","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T03:51:30Z","receivedAt":"2009-02-14T03:51:30Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"From: Jay Soffian \n\nIf --suppress-cc=author then there is no need to unquote the From:\naddress.\n\nSigned-off-by: Jay Soffian \n---\n git-send-email.perl | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex a2e0b94..2a3e3e8 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -958,8 +958,8 @@ foreach my $t (@files) {\n \t\t\t\t$subject = $1;\n \t\t\t}\n \t\t\telsif (/^From:\\s+(.*)$/) {\n+\t\t\t\tnext if $suppress_cc{'author'};\n \t\t\t\t($author, $author_encoding) = unquote_rfc2047($1);\n-\t\t\t\tnext if ($suppress_cc{'author'});\n \t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t\t$1, $_) unless $quiet;\n \t\t\t\tpush @cc, $1;\n-- \n1.6.2.rc0.235.g1319\n"},{"id":"104586","messageId":"1234583491-61260-4-git-send-email-jaysoffian@gmail.com","threadId":"17780","inReplyTo":"1234583491-61260-3-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 3/3] send-email: --suppress-cc improvements","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T03:51:31Z","receivedAt":"2009-02-14T03:51:31Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"From: Jay Soffian \n\nCommit 656482830ddc4a4e2af132fabb118a25190439c2 added the --suppress-cc\noption. However, it made --suppress-cc=sob suppress both SOB lines and\nbody Cc lines (but not header Cc lines), which seems contrary to how\nit is named.\n\nAfter this commit, 'sob' suppresses only SOB lines and --suppress-cc\ntakes two additional values:\n\n * 'body' suppresses both SOB and body Cc lines (i.e. what 'sob'\n used to do).\n\n * 'bodycc' suppresses body Cc lines, but not header Cc lines.\n\nFor backwards compatibility, --no-signed-off-by-cc, acts like 'body'.\n\nAlso update the documentation and add a few tests.\n\nSigned-off-by: Jay Soffian \n---\n\nOkay, so --suppress=cc=sob now acts differently. I can hear the\nbackwards compatibility objection. I disagree. It will only effect a\nuser who does --suppress-cc=sob AND sends out a patch that has Cc: in\nthe message body (commit message). And If I'm not the first user to have\nsent such a message earlier today, I'll bet I'm not much more than the\n10th. \n\nOTOH, --suppress-cc=sob was obviously misnamed in the first place, and I\nwas confused by having send-email pick up the Cc in the body.\n\nSo I think it's worth the small risk of changing what --suppress-cc=sob\ndoes. And as a benefit, we now offer two new values for --suppress-cc,\nand for really-old-timers that are probably using --no-signed-of-by-cc,\nthat says the same.\n\nj.\n\n Documentation/git-send-email.txt | 14 +++++++-----\n git-send-email.perl | 40 +++++++++++++++++++++++--------------\n t/t9001-send-email.sh | 38 ++++++++++++++++++++++++++++++++++-\n 3 files changed, 69 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex ff4aeff..d6af035 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -166,12 +166,14 @@ Automating\n \tSpecify an additional category of recipients to suppress the\n \tauto-cc of. 'self' will avoid including the sender, 'author' will\n \tavoid including the patch author, 'cc' will avoid including anyone\n-\tmentioned in Cc lines in the patch, 'sob' will avoid including\n-\tanyone mentioned in Signed-off-by lines, and 'cccmd' will avoid\n-\trunning the --cc-cmd. 'all' will suppress all auto cc values.\n-\tDefault is the value of 'sendemail.suppresscc' configuration value;\n-\tif that is unspecified, default to 'self' if --suppress-from is\n-\tspecified, as well as 'sob' if --no-signed-off-cc is specified.\n+\tmentioned in Cc lines in the patch header, 'ccbody' will avoid\n+\tincluding anyone mentioned in Cc lines in the patch body (commit\n+\tmessage), 'sob' will avoid including anyone mentioned in Signed-off-by\n+\tlines, and 'cccmd' will avoid running the --cc-cmd. 'body' is\n+\tequivalent to 'sob' + 'ccbody'. 'all' will suppress all auto cc\n+\tvalues. Default is the value of 'sendemail.suppresscc' configuration\n+\tvalue; if that is unspecified, default to 'self' if --suppress-from is\n+\tspecified, as well as 'body' if --no-signed-off-cc is specified.\n \n --[no-]suppress-from::\n \tIf this is set, do not add the From: address to the cc: list.\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 2a3e3e8..2282287 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -68,7 +68,7 @@ git send-email [options] \n Automating:\n --identity * Use the sendemail. options.\n --cc-cmd * Email Cc: via ` \\$patch_path`\n- --suppress-cc * author, self, sob, cccmd, all.\n+ --suppress-cc * author, self, sob, cc, cccmd, body, bodycc, all.\n --[no-]signed-off-by-cc * Send to Cc: and Signed-off-by:\n addresses. Default on.\n --[no-]suppress-from * Send to self. Default off.\n@@ -319,21 +319,28 @@ my(%suppress_cc);\n if (@suppress_cc) {\n \tforeach my $entry (@suppress_cc) {\n \t\tdie \"Unknown --suppress-cc field: '$entry'\\n\"\n-\t\t\tunless $entry =~ /^(all|cccmd|cc|author|self|sob)$/;\n+\t\t\tunless $entry =~ /^(all|cccmd|cc|author|self|sob|body|bodycc)$/;\n \t\t$suppress_cc{$entry} = 1;\n \t}\n }\n \n if ($suppress_cc{'all'}) {\n-\tforeach my $entry (qw (ccmd cc author self sob)) {\n+\tforeach my $entry (qw (ccmd cc author self sob body bodycc)) {\n \t\t$suppress_cc{$entry} = 1;\n \t}\n \tdelete $suppress_cc{'all'};\n }\n \n+if ($suppress_cc{'sob'} && $suppress_cc{'bodycc'}) {\n+\t$suppress_cc{'body'} = 1;\n+}\n+\n # If explicit old-style ones are specified, they trump --suppress-cc.\n $suppress_cc{'self'} = $suppress_from if defined $suppress_from;\n-$suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;\n+# For backwards compatibility, old-style --signed-off-by-cc suppresses\n+# SOB and body Cc lines, whereas --supress-cc=sob suppresses just the SOB\n+# line, but not the body Cc.\n+$suppress_cc{'body'} = !$signed_off_by_cc if defined $signed_off_by_cc;\n \n # Debugging, print out the suppressions.\n if (0) {\n@@ -1005,17 +1012,20 @@ foreach my $t (@files) {\n \t\t}\n \t}\n \t# Now parse the message body\n-\twhile() {\n-\t\t$message .= $_;\n-\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n-\t\t\tnext if ($suppress_cc{'sob'});\n-\t\t\tchomp;\n-\t\t\tmy $c = $2;\n-\t\t\tchomp $c;\n-\t\t\tnext if ($c eq $sender and $suppress_cc{'self'});\n-\t\t\tpush @cc, $c;\n-\t\t\tprintf(\"(sob) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t$c, $_) unless $quiet;\n+\tunless ($suppress_cc{'body'}) {\n+\t\twhile() {\n+\t\t\t$message .= $_;\n+\t\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n+\t\t\t\tchomp;\n+\t\t\t\tmy ($what, $c) = ($1, $2);\n+\t\t\t\tchomp $c;\n+\t\t\t\tnext if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;\n+\t\t\t\tnext if $suppress_cc{'bodycc'} and $what =~ /Cc/i;\n+\t\t\t\tnext if ($c eq $sender and $suppress_cc{'self'});\n+\t\t\t\tpush @cc, $c;\n+\t\t\t\tprintf(\"(body) Adding cc: %s from line '%s'\\n\",\n+\t\t\t\t\t$c, $_) unless $quiet;\n+\t\t\t}\n \t\t}\n \t}\n \tclose F;\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex da54835..d7766f9 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -32,11 +32,11 @@ clean_fake_sendmail() {\n }\n \n test_expect_success 'Extract patches' '\n- patches=`git format-patch --cc=\"One \" --cc=two@example.com -n HEAD^1`\n+ patches=`git format-patch -s --cc=\"One \" --cc=two@example.com -n HEAD^1`\n '\n \n test_expect_success 'Send patches' '\n- git send-email --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n+ git send-email --suppress-cc=sob --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n '\n \n cat >expected <<\\EOF\n@@ -74,6 +74,7 @@ EOF\n test_expect_success 'Show all headers' '\n \tgit send-email \\\n \t\t--dry-run \\\n+\t\t--suppress-cc=sob \\\n \t\t--from=\"Example \" \\\n \t\t--to=to@example.com \\\n \t\t--cc=cc@example.com \\\n@@ -195,6 +196,7 @@ test_expect_success 'sendemail.cc set' '\n \tgit config sendemail.cc cc@example.com &&\n \tgit send-email \\\n \t\t--dry-run \\\n+\t\t--suppress-cc=sob \\\n \t\t--from=\"Example \" \\\n \t\t--to=to@example.com \\\n \t\t--smtp-server relay.example.com \\\n@@ -230,6 +232,38 @@ test_expect_success 'sendemail.cc unset' '\n \tgit config --unset sendemail.cc &&\n \tgit send-email \\\n \t\t--dry-run \\\n+\t\t--suppress-cc=sob \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=to@example.com \\\n+\t\t--smtp-server relay.example.com \\\n+\t\t$patches |\n+\tsed\t-e \"s/^\\(Date:\\).*/\\1 DATE-STRING/\" \\\n+\t\t-e \"s/^\\(Message-Id:\\).*/\\1 MESSAGE-ID-STRING/\" \\\n+\t\t-e \"s/^\\(X-Mailer:\\).*/\\1 X-MAILER-STRING/\" \\\n+\t\t>actual-show-all-headers &&\n+\ttest_cmp expected-show-all-headers actual-show-all-headers\n+'\n+\n+cat >expected-show-all-headers <<\\EOF\n+0001-Second.patch\n+Dry-OK. Log says:\n+Server: relay.example.com\n+MAIL FROM:\n+RCPT TO:\n+From: Example \n+To: to@example.com\n+Subject: [PATCH 1/1] Second.\n+Date: DATE-STRING\n+Message-Id: MESSAGE-ID-STRING\n+X-Mailer: X-MAILER-STRING\n+\n+Result: OK\n+EOF\n+\n+test_expect_success '--suppress-cc=all' '\n+\tgit send-email \\\n+\t\t--dry-run \\\n+\t\t--suppress-cc=all \\\n \t\t--from=\"Example \" \\\n \t\t--to=to@example.com \\\n \t\t--smtp-server relay.example.com \\\n-- \n1.6.2.rc0.235.g1319\n"},{"id":"104589","messageId":"1234589823-67610-1-git-send-email-jaysoffian@gmail.com","threadId":"17780","inReplyTo":"1234583491-61260-4-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 3/3 v2] send-email: --suppress-cc improvements","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T05:37:03Z","receivedAt":"2009-02-14T05:37:03Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"From: Jay Soffian \n\nCommit 656482830ddc4a4e2af132fabb118a25190439c2 added the --suppress-cc\noption. However, it made --suppress-cc=sob suppress both SOB lines and\nbody Cc lines (but not header Cc lines), which seems contrary to how\nit is named.\n\nAfter this commit, 'sob' suppresses only SOB lines and --suppress-cc\ntakes two additional values:\n\n * 'body' suppresses both SOB and body Cc lines (i.e. what 'sob'\n used to do).\n\n * 'bodycc' suppresses body Cc lines, but not header Cc lines.\n\nFor backwards compatibility, --no-signed-off-by-cc, acts like 'body'.\n\nAlso update the documentation and add a few tests.\n\nSigned-off-by: Jay Soffian \n---\nSorry, please ignore the previous 3/3, it had an obvious breakage;\n--suppress-cc=body was causing then entire message body not to be read\nin at all.\n\n Documentation/git-send-email.txt | 14 ++++++++------\n git-send-email.perl | 23 ++++++++++++++++-------\n t/t9001-send-email.sh | 38 ++++++++++++++++++++++++++++++++++++--\n 3 files changed, 60 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex ff4aeff..d6af035 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -166,12 +166,14 @@ Automating\n \tSpecify an additional category of recipients to suppress the\n \tauto-cc of. 'self' will avoid including the sender, 'author' will\n \tavoid including the patch author, 'cc' will avoid including anyone\n-\tmentioned in Cc lines in the patch, 'sob' will avoid including\n-\tanyone mentioned in Signed-off-by lines, and 'cccmd' will avoid\n-\trunning the --cc-cmd. 'all' will suppress all auto cc values.\n-\tDefault is the value of 'sendemail.suppresscc' configuration value;\n-\tif that is unspecified, default to 'self' if --suppress-from is\n-\tspecified, as well as 'sob' if --no-signed-off-cc is specified.\n+\tmentioned in Cc lines in the patch header, 'ccbody' will avoid\n+\tincluding anyone mentioned in Cc lines in the patch body (commit\n+\tmessage), 'sob' will avoid including anyone mentioned in Signed-off-by\n+\tlines, and 'cccmd' will avoid running the --cc-cmd. 'body' is\n+\tequivalent to 'sob' + 'ccbody'. 'all' will suppress all auto cc\n+\tvalues. Default is the value of 'sendemail.suppresscc' configuration\n+\tvalue; if that is unspecified, default to 'self' if --suppress-from is\n+\tspecified, as well as 'body' if --no-signed-off-cc is specified.\n \n --[no-]suppress-from::\n \tIf this is set, do not add the From: address to the cc: list.\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 2a3e3e8..23a55e2 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -68,7 +68,7 @@ git send-email [options] \n Automating:\n --identity * Use the sendemail. options.\n --cc-cmd * Email Cc: via ` \\$patch_path`\n- --suppress-cc * author, self, sob, cccmd, all.\n+ --suppress-cc * author, self, sob, cc, cccmd, body, bodycc, all.\n --[no-]signed-off-by-cc * Send to Cc: and Signed-off-by:\n addresses. Default on.\n --[no-]suppress-from * Send to self. Default off.\n@@ -319,21 +319,28 @@ my(%suppress_cc);\n if (@suppress_cc) {\n \tforeach my $entry (@suppress_cc) {\n \t\tdie \"Unknown --suppress-cc field: '$entry'\\n\"\n-\t\t\tunless $entry =~ /^(all|cccmd|cc|author|self|sob)$/;\n+\t\t\tunless $entry =~ /^(all|cccmd|cc|author|self|sob|body|bodycc)$/;\n \t\t$suppress_cc{$entry} = 1;\n \t}\n }\n \n if ($suppress_cc{'all'}) {\n-\tforeach my $entry (qw (ccmd cc author self sob)) {\n+\tforeach my $entry (qw (ccmd cc author self sob body bodycc)) {\n \t\t$suppress_cc{$entry} = 1;\n \t}\n \tdelete $suppress_cc{'all'};\n }\n \n+if ($suppress_cc{'sob'} && $suppress_cc{'bodycc'}) {\n+\t$suppress_cc{'body'} = 1;\n+}\n+\n # If explicit old-style ones are specified, they trump --suppress-cc.\n $suppress_cc{'self'} = $suppress_from if defined $suppress_from;\n-$suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;\n+# For backwards compatibility, old-style --signed-off-by-cc suppresses\n+# SOB and body Cc lines, whereas --supress-cc=sob suppresses just the SOB\n+# line, but not the body Cc.\n+$suppress_cc{'body'} = !$signed_off_by_cc if defined $signed_off_by_cc;\n \n # Debugging, print out the suppressions.\n if (0) {\n@@ -1007,14 +1014,16 @@ foreach my $t (@files) {\n \t# Now parse the message body\n \twhile() {\n \t\t$message .= $_;\n+\t\tnext if $suppress_cc{'body'};\n \t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n-\t\t\tnext if ($suppress_cc{'sob'});\n \t\t\tchomp;\n-\t\t\tmy $c = $2;\n+\t\t\tmy ($what, $c) = ($1, $2);\n \t\t\tchomp $c;\n+\t\t\tnext if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;\n+\t\t\tnext if $suppress_cc{'bodycc'} and $what =~ /Cc/i;\n \t\t\tnext if ($c eq $sender and $suppress_cc{'self'});\n \t\t\tpush @cc, $c;\n-\t\t\tprintf(\"(sob) Adding cc: %s from line '%s'\\n\",\n+\t\t\tprintf(\"(body) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t$c, $_) unless $quiet;\n \t\t}\n \t}\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex da54835..d7766f9 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -32,11 +32,11 @@ clean_fake_sendmail() {\n }\n \n test_expect_success 'Extract patches' '\n- patches=`git format-patch --cc=\"One \" --cc=two@example.com -n HEAD^1`\n+ patches=`git format-patch -s --cc=\"One \" --cc=two@example.com -n HEAD^1`\n '\n \n test_expect_success 'Send patches' '\n- git send-email --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n+ git send-email --suppress-cc=sob --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n '\n \n cat >expected <<\\EOF\n@@ -74,6 +74,7 @@ EOF\n test_expect_success 'Show all headers' '\n \tgit send-email \\\n \t\t--dry-run \\\n+\t\t--suppress-cc=sob \\\n \t\t--from=\"Example \" \\\n \t\t--to=to@example.com \\\n \t\t--cc=cc@example.com \\\n@@ -195,6 +196,7 @@ test_expect_success 'sendemail.cc set' '\n \tgit config sendemail.cc cc@example.com &&\n \tgit send-email \\\n \t\t--dry-run \\\n+\t\t--suppress-cc=sob \\\n \t\t--from=\"Example \" \\\n \t\t--to=to@example.com \\\n \t\t--smtp-server relay.example.com \\\n@@ -230,6 +232,38 @@ test_expect_success 'sendemail.cc unset' '\n \tgit config --unset sendemail.cc &&\n \tgit send-email \\\n \t\t--dry-run \\\n+\t\t--suppress-cc=sob \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=to@example.com \\\n+\t\t--smtp-server relay.example.com \\\n+\t\t$patches |\n+\tsed\t-e \"s/^\\(Date:\\).*/\\1 DATE-STRING/\" \\\n+\t\t-e \"s/^\\(Message-Id:\\).*/\\1 MESSAGE-ID-STRING/\" \\\n+\t\t-e \"s/^\\(X-Mailer:\\).*/\\1 X-MAILER-STRING/\" \\\n+\t\t>actual-show-all-headers &&\n+\ttest_cmp expected-show-all-headers actual-show-all-headers\n+'\n+\n+cat >expected-show-all-headers <<\\EOF\n+0001-Second.patch\n+Dry-OK. Log says:\n+Server: relay.example.com\n+MAIL FROM:\n+RCPT TO:\n+From: Example \n+To: to@example.com\n+Subject: [PATCH 1/1] Second.\n+Date: DATE-STRING\n+Message-Id: MESSAGE-ID-STRING\n+X-Mailer: X-MAILER-STRING\n+\n+Result: OK\n+EOF\n+\n+test_expect_success '--suppress-cc=all' '\n+\tgit send-email \\\n+\t\t--dry-run \\\n+\t\t--suppress-cc=all \\\n \t\t--from=\"Example \" \\\n \t\t--to=to@example.com \\\n \t\t--smtp-server relay.example.com \\\n-- \n1.6.2.rc0.238.g0c1fe\n"},{"id":"104592","messageId":"76718490902132150w59d48581ke1c6394809bf810a@mail.gmail.com","threadId":"17780","inReplyTo":"1234583491-61260-3-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 2/3] send-email: don't call unquote_rfc2047 unnecessarily","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T05:50:00Z","receivedAt":"2009-02-14T05:50:00Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 10:51 PM, Jay Soffian wrote:\n> From: Jay Soffian \n>\n> If --suppress-cc=author then there is no need to unquote the From:\n> address.\n>\n> Signed-off-by: Jay Soffian \n> ---\n> git-send-email.perl | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index a2e0b94..2a3e3e8 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -958,8 +958,8 @@ foreach my $t (@files) {\n> $subject = $1;\n> }\n> elsif (/^From:\\s+(.*)$/) {\n> + next if $suppress_cc{'author'};\n> ($author, $author_encoding) = unquote_rfc2047($1);\n> - next if ($suppress_cc{'author'});\n> printf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n> $1, $_) unless $quiet;\n> push @cc, $1;\n\nCrap. Ignore this one from the series. $author is used later and we\nhave to parse it regardless. This is brittle code, but I should've\nrealize it.\n\nj.\n"},{"id":"104596","messageId":"1234592519-71980-1-git-send-email-jaysoffian@gmail.com","threadId":"17780","inReplyTo":"1234583491-61260-2-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 1/3 v2] send-email: correct logic error with --suppress-cc=cc","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T06:21:59Z","receivedAt":"2009-02-14T06:21:59Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"--suppress-cc=cc is supposed to suppress harvesting addresses from any\nCc: lines. However, in the case where the Cc: line contained the sender,\nit would only suppress if --suppress-cc=self.\n\nSigned-off-by: Jay Soffian \n---\nAnd here's a re-roll of this one. Sheesh, that logic was too subtle for\nmy brain. I think it's a lot more obvious what is going on after this\npatch.\n\nI apologize that this thread is getting so dense. Here it is on gmane\nwhich may help:\n\n http://thread.gmane.org/gmane.comp.version-control.git/109783\n\nj.\n\n git-send-email.perl | 13 ++++++-------\n 1 files changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex cde294c..cef32da 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -959,18 +959,17 @@ foreach my $t (@files) {\n \t\t\t}\n \t\t\telsif (/^From:\\s+(.*)$/) {\n \t\t\t\t($author, $author_encoding) = unquote_rfc2047($1);\n-\t\t\t\tnext if ($suppress_cc{'author'});\n+\t\t\t\tnext if $suppress_cc{'author'};\n+\t\t\t\tnext if $suppress_cc{'self'} and $author eq $sender;\n \t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t\t$1, $_) unless $quiet;\n \t\t\t\tpush @cc, $1;\n \t\t\t}\n \t\t\telsif (/^Cc:\\s+(.*)$/) {\n+\t\t\t\tnext if $suppress_cc{'cc'};\n \t\t\t\tforeach my $addr (parse_address_line($1)) {\n-\t\t\t\t\tif (unquote_rfc2047($addr) eq $sender) {\n-\t\t\t\t\t\tnext if ($suppress_cc{'self'});\n-\t\t\t\t\t} else {\n-\t\t\t\t\t\tnext if ($suppress_cc{'cc'});\n-\t\t\t\t\t}\n+\t\t\t\t\tnext if $suppress_cc{'self'} and\n+\t\t\t\t\t\tunquote_rfc2047($addr) eq $sender;\n \t\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t\t\t$addr, $_) unless $quiet;\n \t\t\t\t\tpush @cc, $addr;\n@@ -1038,7 +1037,7 @@ foreach my $t (@files) {\n \t\t\tor die \"(cc-cmd) failed to close pipe to '$cc_cmd'\";\n \t}\n \n-\tif (defined $author) {\n+\tif (defined $author and $author ne $sender) {\n \t\t$message = \"From: $author\\n\\n$message\";\n \t\tif (defined $author_encoding) {\n \t\t\tif ($has_content_type) {\n-- \n1.6.2.rc0.238.g0c1fe\n"},{"id":"104598","messageId":"7vzlgpzfru.fsf@gitster.siamese.dyndns.org","threadId":"17780","inReplyTo":"1234589823-67610-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 3/3 v2] send-email: --suppress-cc improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T06:36:21Z","receivedAt":"2009-02-14T06:36:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> From: Jay Soffian \n>\n> Commit 656482830ddc4a4e2af132fabb118a25190439c2 added the --suppress-cc\n> option. However, it made --suppress-cc=sob suppress both SOB lines and\n> body Cc lines (but not header Cc lines), which seems contrary to how\n> it is named.\n>\n> After this commit, 'sob' suppresses only SOB lines and --suppress-cc\n> takes two additional values:\n\nI know it's just wording, but it always makes my skin tingle when I see\n\"Before this commit, blah, after this commit bah.\" Maybe it's just me?\nThe logic flows more naturally if you say \"X does Y. It is not good for\nsuch and such reasons. This patch changes it to do Z.\", at least to me.\n\n> * 'body' suppresses both SOB and body Cc lines (i.e. what 'sob'\n> used to do).\n>\n> * 'bodycc' suppresses body Cc lines, but not header Cc lines.\n>\n> For backwards compatibility, --no-signed-off-by-cc, acts like 'body'.\n\nI had to read this sentence three time. Giving --no-signed-off-by-cc is\nthe same as giving --suppress-cc=body, meaning it suppresses both S-o-b\nand in-body Cc addresses?\n\nIt looks very weird to say \"For backwards compatibility,\" immediately\nafter you said \"screw backward compatibility\" by changing the meaning of\n\"sob\" without even justifying why it is a good idea in the commit log\nmessage.\n\nYou are changing the semantics of \"sob\" because you believe the new\ndefinition is much saner (and I happen to agree with you but that is\nbesides the point). When the user says --no-signed-off-by-cc, the user\nasks not to Cc: people who have sign-offs in the log message; I'd say it\nwould be more natural to screw the backward compatibility here as well and\ngive it a saner and more natural semantics of suppressing S-o-b but not\nin-body Cc. No?\n\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index ff4aeff..d6af035 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -166,12 +166,14 @@ Automating\n> \tSpecify an additional category of recipients to suppress the\n> \tauto-cc of. 'self' will avoid including the sender, 'author' will\n> \tavoid including the patch author, 'cc' will avoid including anyone\n> +\tmentioned in Cc lines in the patch header, 'ccbody' will avoid\n> +\tincluding anyone mentioned in Cc lines in the patch body (commit\n> +\tmessage), 'sob' will avoid including anyone mentioned in Signed-off-by\n> +\tlines, and 'cccmd' will avoid running the --cc-cmd. 'body' is\n> +\tequivalent to 'sob' + 'ccbody'. 'all' will suppress all auto cc\n> +\tvalues. Default is the value of 'sendemail.suppresscc' configuration\n> +\tvalue; if that is unspecified, default to 'self' if --suppress-from is\n> +\tspecified, as well as 'body' if --no-signed-off-cc is specified.\n\nPerhaps with this many choices, it would be better to rewrite this into a\nlist form.\n\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 2a3e3e8..23a55e2 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -319,21 +319,28 @@ my(%suppress_cc);\n> if (@suppress_cc) {\n> \tforeach my $entry (@suppress_cc) {\n> \t\tdie \"Unknown --suppress-cc field: '$entry'\\n\"\n> -\t\t\tunless $entry =~ /^(all|cccmd|cc|author|self|sob)$/;\n> +\t\t\tunless $entry =~ /^(all|cccmd|cc|author|self|sob|body|bodycc)$/;\n> \t\t$suppress_cc{$entry} = 1;\n> \t}\n> }\n> \n> if ($suppress_cc{'all'}) {\n> -\tforeach my $entry (qw (ccmd cc author self sob)) {\n> +\tforeach my $entry (qw (ccmd cc author self sob body bodycc)) {\n> \t\t$suppress_cc{$entry} = 1;\n> \t}\n> \tdelete $suppress_cc{'all'};\n> }\n> \n> +if ($suppress_cc{'sob'} && $suppress_cc{'bodycc'}) {\n> +\t$suppress_cc{'body'} = 1;\n> +}\n\nHmmm.\n\nI wonder if doing this the other way around, and treat 'body' as a mere\nshorthand of setting 'sob' and 'bodycc', just like we do to 'all' (which\nis just a shorthand to set all of them), and delete $suppress_cc{'body'},\nwould make the program much less error prone in the longer term, because\nthen each test needs to check the independent flags and do not have to\nworry about a synthetic flag 'body'.\n\nIt's just a common sense futureproofing. More of your code will resist\npossible changes to the meaning of 'body' in the future that way..\n\n> # Debugging, print out the suppressions.\n> if (0) {\n> @@ -1007,14 +1014,16 @@ foreach my $t (@files) {\n> \t# Now parse the message body\n> \twhile() {\n> \t\t$message .= $_;\n> +\t\tnext if $suppress_cc{'body'};\n> \t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n> \t\t\tchomp;\n> +\t\t\tmy ($what, $c) = ($1, $2);\n> \t\t\tchomp $c;\n> +\t\t\tnext if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;\n> +\t\t\tnext if $suppress_cc{'bodycc'} and $what =~ /Cc/i;\n> \t\t\tnext if ($c eq $sender and $suppress_cc{'self'});\n> \t\t\tpush @cc, $c;\n> +\t\t\tprintf(\"(body) Adding cc: %s from line '%s'\\n\",\n> \t\t\t\t$c, $_) unless $quiet;\n\nLooks sane.\n\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index da54835..d7766f9 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -32,11 +32,11 @@ clean_fake_sendmail() {\n> }\n> \n> test_expect_success 'Extract patches' '\n> - patches=`git format-patch --cc=\"One \" --cc=two@example.com -n HEAD^1`\n> + patches=`git format-patch -s --cc=\"One \" --cc=two@example.com -n HEAD^1`\n> '\n> \n> test_expect_success 'Send patches' '\n> - git send-email --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n> + git send-email --suppress-cc=sob --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n> '\n\nLooks sane.\n\n> @@ -74,6 +74,7 @@ EOF\n> test_expect_success 'Show all headers' '\n> \tgit send-email \\\n> \t\t--dry-run \\\n> +\t\t--suppress-cc=sob \\\n> \t\t--from=\"Example \" \\\n> \t\t--to=to@example.com \\\n> \t\t--cc=cc@example.com \\\n\nIt is somewhat troublesome that only sob suppression is tested, output\nfrom messages that do have in-body S-o-b/Cc without any --suppress-cc is\nnot tested anymore, and none of the new ones you added is tested. You can\nbuild confidence in the working of the suppression that way, but you would\nalso want to test that your new code is not over-suppressing things by\nmistake.\n\nWhen writing new tests, people tend to concentrate so much on showing the\nnew feature works in writing positive tests to forget that negative tests\nare equally important.\n"},{"id":"104602","messageId":"7viqndzf0o.fsf@gitster.siamese.dyndns.org","threadId":"17780","inReplyTo":"1234592519-71980-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 1/3 v2] send-email: correct logic error with --suppress-cc=cc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T06:52:39Z","receivedAt":"2009-02-14T06:52:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> --suppress-cc=cc is supposed to suppress harvesting addresses from any\n> Cc: lines. However, in the case where the Cc: line contained the sender,\n> it would only suppress if --suppress-cc=self.\n\nPerhaps I am personally slightly in favor of your interpretation, but the\ncurrent behaviour makes sort of sense, too. To everybody, himself is\nalways special (and that is why we have 'self'); I do not think it is\nentirely unreasonable if --suppress-cc=cc meant remove people from Cc list\nbut I am special.\n\n> And here's a re-roll of this one. Sheesh, that logic was too subtle for\n> my brain.\n\nPerhaps it is easier to clarify the goal by updating the tests to define\nwhat needs to happen first? Working from tests often makes the end result\nmuch more robust than randomly modifying the existing code here and there\nuntil it happens to start doing something you think it should do.\n"},{"id":"104605","messageId":"76718490902132315l1a9d9463n255cdfe5cce15d22@mail.gmail.com","threadId":"17780","inReplyTo":"7viqndzf0o.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3 v2] send-email: correct logic error with --suppress-cc=cc","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T07:15:45Z","receivedAt":"2009-02-14T07:15:45Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, Feb 14, 2009 at 1:52 AM, Junio C Hamano wrote:\n> Perhaps it is easier to clarify the goal by updating the tests to define\n> what needs to happen first? Working from tests often makes the end result\n> much more robust than randomly modifying the existing code here and there\n> until it happens to start doing something you think it should do.\n\nIn this case the problem is that the existing code had no tests, so I wasn't\nquite sure what it was supposed to do, and I divined the wrong thing from it.\n\nBut now that I understand what it should do, I'll add tests.\n\nWho let the original code in w/o tests, hmm? :-)\n\nj.\n"},{"id":"104608","messageId":"7vvdrdxyh7.fsf@gitster.siamese.dyndns.org","threadId":"17780","inReplyTo":"76718490902132315l1a9d9463n255cdfe5cce15d22@mail.gmail.com","subject":"Re: [PATCH 1/3 v2] send-email: correct logic error with --suppress-cc=cc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T07:35:16Z","receivedAt":"2009-02-14T07:35:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> Who let the original code in w/o tests, hmm? :-)\n\nReviewers and list participants, including you.\n"},{"id":"104611","messageId":"7v63jdxwl9.fsf@gitster.siamese.dyndns.org","threadId":"17780","inReplyTo":"76718490902131642y62b81bfat129cfbeda7957ff8@mail.gmail.com","subject":"Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T08:16:02Z","receivedAt":"2009-02-14T08:16:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> It goes like this:\n>\n> Me: Notice wart\n> Me: Cut wart off (i.e. submit patch)\n> Reviewer: Sorry, you didn't cut that wart off to my standards, unless\n> you can do it better, I'd rather have the wart.\n>\n> I dunno, maybe I'm too sensitive and don't have the fortitude for\n> contributing to git.\n\nDifferent people have different priorities and judging criteria.\n\nTo some authors, code is the only thing that matters and they say a crappy\ncommit log message is Ok if the code works. I actually do not even read\nthe patch if the commit log message does not clearly express what problem\nthe author is trying to address, because I always have other patches to\nattend to, and if the author cannot formulate his thought clearly in the\ncommit log message, it is likely that the code doesn't, either, and even\nif it does, the code speaks only about how it does what it does, and not\nmuch about _why_ it is so, which is the most important thing down the\nroad.\n\nSome people comment more on readability of the patch and format of the\ncode while some others let them pass and concentrate on performance and\ncorrectness.\n\nAll of them are important, and reviewers, together with the original\nauthor, complement each other's weakness to work toward a better system.\n\nThe first thing to learn is to tell the difference between \"you didn't cut\nthat wart off to my standards, unless you can do it better, I'd rather\nhave the wart.\" and \"you claim you have cut the wart off, but that's minor\ncompared to the wart you are missing here which can be cut at the same\ntime without too much effort; let's do so at the same time before we all\nforget\".\n\nThe review comments I read on this list are more often the latter, but I\ncan certainly understand some authors, being so excited about and fond of\nhis own creation, mistake it as the former. Two tricks I learned to avoid\nthat trap while working on git, and especially when git was still young\nand I was a contributor, was to sleep on things, and to always consider\nthe possibility that I _might_ be wrong. The latter takes some practice,\nand I admit I still haven't mastered it yet, but I am trying.\n"},{"id":"104650","messageId":"200902141757.17983.trast@student.ethz.ch","threadId":"17780","inReplyTo":"1234583491-61260-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] send-email: handle multiple Cc addresses when reading mbox message","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T16:57:14Z","receivedAt":"2009-02-14T16:57:14Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jay Soffian wrote:\n> From: Jay Soffian \n> \n> On Fri, Feb 13, 2009 at 6:32 PM, Thomas Rast wrote:\n> > Jay Soffian wrote:\n> >> - if (/^(Signed-off-by|Cc): (.*)$/i) {\n> >> - next if ($suppress_cc{'sob'});\n> > [...]\n> >> + if (/^(Signed-off-by|Cc): (.*)$/i) {\n> >> + next if ($suppress_cc{'sob'});\n> >\n> > Doesn't this actually look like a long-standing send-email bug? Since\n> > 6564828 (git-send-email: Generalize auto-cc recipient mechanism.,\n> > 2007-12-25) they should go in separate categories, but the above lines\n> > were just translated from the old $signed_off_cc setting. It seems\n> > they should distinguish between SOB and Cc.\n> \n> This is fixed by the last patch in this series. While I was working on\n> it I noticed two other minor issues, which is the first two patches.\n\nThanks for the fix, and sorry if this was the comment that inspired\nyour venting earlier. I should've tried a patch myself, but it was\nalready past midnight. I'll send a slightly improved version of 3/3\nshortly.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"}]}