{"thread":{"id":"33937","subject":"[PATCH 0/6] git send-email suppress-cc=self fixes","startedAt":"2013-05-26T14:40:52Z","lastAt":"2013-05-30T04:51:54Z","messageCount":12,"participants":["Michael S. Tsirkin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"218558","messageId":"1369579187-27573-1-git-send-email-mst@redhat.com","threadId":"33937","inReplyTo":null,"subject":"[PATCH 0/6] git send-email suppress-cc=self fixes","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-05-26T14:40:52Z","receivedAt":"2013-05-26T14:40:52Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"This includes bugfixes related to handling of --suppress-cc=self\nflag. Tests are also included.\n\n-- \nMST\n"},{"id":"218559","messageId":"1369579187-27573-2-git-send-email-mst@redhat.com","threadId":"33937","inReplyTo":"1369579187-27573-1-git-send-email-mst@redhat.com","subject":"[PATCH 1/6] t/send-email.sh: add test for suppress self","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-05-26T14:40:56Z","receivedAt":"2013-05-26T14:40:56Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n t/t9001-send-email.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 41 insertions(+)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex ebd5c5d..36ecf73 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -171,6 +171,47 @@ Result: OK\n EOF\n \"\n \n+test_suppress_self () {\n+\t\ttest_commit $3 &&\n+\t\ttest_when_finished \"git reset --hard HEAD^\" &&\n+\t\t{\n+\t\t\techo \"#!$SHELL_PATH\"\n+\t\t\techo sed -n -e s/^cccmd--//p \\\"\\$1\\\"\n+\t\t} > cccmd-sed &&\n+\t\tchmod +x cccmd-sed &&\n+\t\tgit commit --amend --author=\"$1 <$2>\" -F - << EOF && \\\n+\t\tclean_fake_sendmail && \\\n+\t\techo suppress-self-$3.patch > /dev/tty && \\\n+\t\tgit format-patch --stdout -1 >suppress-self-$3.patch && \\\n+\t\tgit send-email --from=\"$1 <$2>\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--cc-cmd=./cccmd-sed \\\n+\t\t--suppress-cc=self \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\tsuppress-self-$3.patch && \\\n+\t\tmv msgtxt1 msgtxt1-$3 && \\\n+\t\tsed -e '/^$/q' msgtxt1-$3 > msghdr1-$3 && \\\n+\t\trm -f expected-no-cc-$3 && \\\n+\t\ttouch expected-no-cc-$3 && \\\n+\t\tgrep '^Cc:' msghdr1-$3 > actual-no-cc-$3 && \\\n+\t\ttest_cmp expected-no-cc-$3 actual-no-cc-$3\n+test suppress-cc.self $3 with name $1 email $2\n+\n+$3\n+\n+cccmd--\"$1\" <$2>\n+\n+Cc: \"$1\" <$2>\n+Cc: $1 <$2>\n+Signed-off-by: \"$1\" <$2>\n+Signed-off-by: $1 <$2>\n+EOF\n+}\n+\n+test_expect_success $PREREQ 'self name is suppressed' \"\n+\ttest_suppress_self 'A U Thor' 'author@redhat.com' 'self_name_suppressed'\n+\"\n+\n test_expect_success $PREREQ 'Show all headers' '\n \tgit send-email \\\n \t\t--dry-run \\\n-- \nMST\n"},{"id":"218560","messageId":"1369579187-27573-3-git-send-email-mst@redhat.com","threadId":"33937","inReplyTo":"1369579187-27573-1-git-send-email-mst@redhat.com","subject":"[PATCH 2/6] send-email: fix suppress-cc=self on cccmd","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-05-26T14:40:59Z","receivedAt":"2013-05-26T14:40:59Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"When cccmd is used, old-style suppress-from filter\nis applied by the newer suppress-cc=self isn't.\nFix this up.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n git-send-email.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex bd13cc8..a138615 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1462,7 +1462,7 @@ sub recipients_cmd {\n \t\t$address =~ s/^\\s*//g;\n \t\t$address =~ s/\\s*$//g;\n \t\t$address = sanitize_address($address);\n-\t\tnext if ($address eq $sanitized_sender and $suppress_from);\n+\t\tnext if ($address eq $sender and $suppress_cc{'self'});\n \t\tpush @addresses, $address;\n \t\tprintf(\"($prefix) Adding %s: %s from: '%s'\\n\",\n \t\t       $what, $address, $cmd) unless $quiet;\n-- \nMST\n"},{"id":"218561","messageId":"1369579187-27573-4-git-send-email-mst@redhat.com","threadId":"33937","inReplyTo":"1369579187-27573-1-git-send-email-mst@redhat.com","subject":"[PATCH 3/6] t/send-email: test suppress-cc=self on cccmd","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-05-26T14:41:01Z","receivedAt":"2013-05-26T14:41:01Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Check that suppress-cc=self works when applied\nto output of cccmd.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n t/t9001-send-email.sh | 50 +++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 33 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 36ecf73..8aa55f8 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -179,37 +179,53 @@ test_suppress_self () {\n \t\t\techo sed -n -e s/^cccmd--//p \\\"\\$1\\\"\n \t\t} > cccmd-sed &&\n \t\tchmod +x cccmd-sed &&\n-\t\tgit commit --amend --author=\"$1 <$2>\" -F - << EOF && \\\n-\t\tclean_fake_sendmail && \\\n-\t\techo suppress-self-$3.patch > /dev/tty && \\\n-\t\tgit format-patch --stdout -1 >suppress-self-$3.patch && \\\n-\t\tgit send-email --from=\"$1 <$2>\" \\\n+\t\tgit commit --amend --author=\"$1 <$2>\" -F - && \n+\t\tclean_fake_sendmail && \n+\t\tgit format-patch --stdout -1 >suppress-self-$3.patch && \n+\t\tgit send-email --from=\"$1 <$2>\"  \\\n \t\t--to=nobody@example.com \\\n \t\t--cc-cmd=./cccmd-sed \\\n \t\t--suppress-cc=self \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n-\t\tsuppress-self-$3.patch && \\\n-\t\tmv msgtxt1 msgtxt1-$3 && \\\n-\t\tsed -e '/^$/q' msgtxt1-$3 > msghdr1-$3 && \\\n-\t\trm -f expected-no-cc-$3 && \\\n-\t\ttouch expected-no-cc-$3 && \\\n-\t\tgrep '^Cc:' msghdr1-$3 > actual-no-cc-$3 && \\\n-\t\ttest_cmp expected-no-cc-$3 actual-no-cc-$3\n-test suppress-cc.self $3 with name $1 email $2\n+\t\tsuppress-self-$3.patch && \n+\t\tmv msgtxt1 msgtxt1-$3 && \n+\t\tsed -e '/^$/q' msgtxt1-$3 > msghdr1-$3 && \n+\t\trm -f expected-no-cc-$3 && \n+\t\ttouch expected-no-cc-$3 && \n+\t\t(grep '^Cc:' msghdr1-$3 > actual-no-cc-$3;\n+\t\t test_cmp expected-no-cc-$3 actual-no-cc-$3)\n+}\n+\n+test_suppress_self_unquoted () {\n+test_suppress_self \"$1\" \"$2\" \"unquoted-$3\" << EOF\n+\t\ttest suppress-cc.self unquoted-$3 with name $1 email $2\n+unquoted-$3\n+\n+cccmd--$1 <$2>\n+\n+Cc: $1 <$2>\n+Signed-off-by: $1 <$2>\n+EOF\n+}\n+\n+test_suppress_self_quoted () {\n+test_suppress_self \"$1\" \"$2\" \"quoted-$3\" << EOF\n+test suppress-cc.self quoted-$3 with name $1 email $2\n \n-$3\n+quoted-$3\n \n cccmd--\"$1\" <$2>\n \n-Cc: \"$1\" <$2>\n Cc: $1 <$2>\n-Signed-off-by: \"$1\" <$2>\n+Cc: \"$1\" <$2>\n Signed-off-by: $1 <$2>\n+Signed-off-by: \"$1\" <$2>\n EOF\n }\n \n+\n test_expect_success $PREREQ 'self name is suppressed' \"\n-\ttest_suppress_self 'A U Thor' 'author@redhat.com' 'self_name_suppressed'\n+\ttest_suppress_self_unquoted 'A U Thor' 'author@redhat.com' 'self_name_suppressed'\n \"\n \n test_expect_success $PREREQ 'Show all headers' '\n-- \nMST\n"},{"id":"218563","messageId":"1369579187-27573-5-git-send-email-mst@redhat.com","threadId":"33937","inReplyTo":"1369579187-27573-1-git-send-email-mst@redhat.com","subject":"[PATCH 4/6] send-email: make --suppress-cc=self sanitize input","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-05-26T14:41:04Z","receivedAt":"2013-05-26T14:41:04Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"--suppress-cc=self fails to filter sender address in many cases where it\nneeds to be sanitized in some way, for example quoted:\n\"A U. Thor\" <author@example.com>\nTo fix, make send-email sanitize both sender and the address it is\ncompared against.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n git-send-email.perl | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex a138615..92df393 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -760,6 +760,8 @@ if (!defined $sender) {\n \t$sender = $repoauthor || $repocommitter || '';\n }\n \n+$sender = sanitize_address($sender);\n+\n my $prompting = 0;\n if (!@initial_to && !defined $to_cmd) {\n \tmy $to = ask(\"Who should the emails be sent to (if any)? \",\n@@ -1113,10 +1115,9 @@ sub send_message {\n \tif ($cc ne '') {\n \t\t$ccline = \"\\nCc: $cc\";\n \t}\n-\tmy $sanitized_sender = sanitize_address($sender);\n \tmake_message_id() unless defined($message_id);\n \n-\tmy $header = \"From: $sanitized_sender\n+\tmy $header = \"From: $sender\n To: $to${ccline}\n Subject: $subject\n Date: $date\n@@ -1133,7 +1134,7 @@ X-Mailer: git-send-email $gitversion\n \t}\n \n \tmy @sendmail_parameters = ('-i', @recipients);\n-\tmy $raw_from = $sanitized_sender;\n+\tmy $raw_from = $sender;\n \tif (defined $envelope_sender && $envelope_sender ne \"auto\") {\n \t\t$raw_from = $envelope_sender;\n \t}\n@@ -1308,8 +1309,9 @@ foreach my $t (@files) {\n \t\t\t}\n \t\t\telsif (/^From:\\s+(.*)$/i) {\n \t\t\t\t($author, $author_encoding) = unquote_rfc2047($1);\n+\t\t\t\tmy $sauthor = sanitize_address($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\tnext if $suppress_cc{'self'} and $sauthor 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@@ -1323,7 +1325,9 @@ foreach my $t (@files) {\n \t\t\t}\n \t\t\telsif (/^Cc:\\s+(.*)$/i) {\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\tmy $qaddr = unquote_rfc2047($addr);\n+\t\t\t\t\tmy $saddr = sanitize_address($qaddr);\n+\t\t\t\t\tif ($saddr 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@@ -1370,7 +1374,8 @@ foreach my $t (@files) {\n \t\t\tchomp;\n \t\t\tmy ($what, $c) = ($1, $2);\n \t\t\tchomp $c;\n-\t\t\tif ($c eq $sender) {\n+\t\t\tmy $sc = sanitize_address($c);\n+\t\t\tif ($sc eq $sender) {\n \t\t\t\tnext if ($suppress_cc{'self'});\n \t\t\t} else {\n \t\t\t\tnext if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;\n@@ -1454,7 +1459,6 @@ foreach my $t (@files) {\n sub recipients_cmd {\n \tmy ($prefix, $what, $cmd, $file) = @_;\n \n-\tmy $sanitized_sender = sanitize_address($sender);\n \tmy @addresses = ();\n \topen my $fh, \"-|\", \"$cmd \\Q$file\\E\"\n \t    or die \"($prefix) Could not execute '$cmd'\";\n-- \nMST\n"},{"id":"218562","messageId":"1369579187-27573-6-git-send-email-mst@redhat.com","threadId":"33937","inReplyTo":"1369579187-27573-1-git-send-email-mst@redhat.com","subject":"[PATCH 5/6] t/send-email: add test with quoted sender","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-05-26T14:41:07Z","receivedAt":"2013-05-26T14:41:07Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"add test where sender address needs to be quoted.\nMake sure --suppress-cc=self works well in this case.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n t/t9001-send-email.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 8aa55f8..0ab4056 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -228,6 +228,10 @@ test_expect_success $PREREQ 'self name is suppressed' \"\n \ttest_suppress_self_unquoted 'A U Thor' 'author@redhat.com' 'self_name_suppressed'\n \"\n \n+test_expect_success $PREREQ 'quoted self name is suppressed' \"\n+\ttest_suppress_self_quoted 'A U. Thor' 'author@redhat.com' 'self_name_suppressed'\n+\"\n+\n test_expect_success $PREREQ 'Show all headers' '\n \tgit send-email \\\n \t\t--dry-run \\\n-- \nMST\n"},{"id":"218564","messageId":"1369579187-27573-7-git-send-email-mst@redhat.com","threadId":"33937","inReplyTo":"1369579187-27573-1-git-send-email-mst@redhat.com","subject":"[PATCH 6/6] t/send-email: test suppress-cc=self with non-ascii","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-05-26T14:41:11Z","receivedAt":"2013-05-26T14:41:11Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"test suppress-cc=self when sender is non-acsii\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n t/t9001-send-email.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 0ab4056..66ebb1e 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -232,6 +232,10 @@ test_expect_success $PREREQ 'quoted self name is suppressed' \"\n \ttest_suppress_self_quoted 'A U. Thor' 'author@redhat.com' 'self_name_suppressed'\n \"\n \n+test_expect_success $PREREQ 'non-ascii self name is suppressed' \"\n+\ttest_suppress_self_quoted 'Füñný Nâmé' 'odd_?=mail@example.com' 'non_ascii_self_suppressed'\n+\"\n+\n test_expect_success $PREREQ 'Show all headers' '\n \tgit send-email \\\n \t\t--dry-run \\\n-- \nMST\n"},{"id":"218898","messageId":"7vy5axr0mz.fsf@alter.siamese.dyndns.org","threadId":"33937","inReplyTo":"1369579187-27573-2-git-send-email-mst@redhat.com","subject":"Re: [PATCH 1/6] t/send-email.sh: add test for suppress self","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-29T20:28:52Z","receivedAt":"2013-05-29T20:28:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" <mst@redhat.com> writes:\n\n> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> ---\n\nThanks.\n\n>  t/t9001-send-email.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 41 insertions(+)\n>\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index ebd5c5d..36ecf73 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -171,6 +171,47 @@ Result: OK\n>  EOF\n>  \"\n>  \n> +test_suppress_self () {\n> +\t\ttest_commit $3 &&\n> +\t\ttest_when_finished \"git reset --hard HEAD^\" &&\n> +\t\t{\n> +\t\t\techo \"#!$SHELL_PATH\"\n> +\t\t\techo sed -n -e s/^cccmd--//p \\\"\\$1\\\"\n> +\t\t} > cccmd-sed &&\n> +\t\tchmod +x cccmd-sed &&\n\nWe can use write_script for this kind of thing, I think.\n\n> +\t\tgit commit --amend --author=\"$1 <$2>\" -F - << EOF && \\\n\nHmm,...  everything below this function is fed as the standard input\nto \"git commit\" as its updated log message (i.e. \"--amend -F -\")?\n\nPuzzled...\n\nThe EOF I can find is at the very bottom of this function, so there\nis no \"next command\" that && at the end of the above line is\ncascading the control to.\n\nDoubly puzzled...\n\nIn any case, please do not add \" \\\" at the end of line when the line\nends one command and \"&&\" at the end of line clearly tells the shell\nthat you haven't stopped talking yet.\n\n> +\t\tclean_fake_sendmail && \\\n> +\t\techo suppress-self-$3.patch > /dev/tty && \\\n\nDo we always have /dev/tty?  If this is a leftover debugging, please\nremove it.  If redirecting it to >&2 does not upset what the test\ndoes, that is good, too (you can run the test with -v option to view\nthe output).\n\n> +\t\tgit format-patch --stdout -1 >suppress-self-$3.patch && \\\n> +\t\tgit send-email --from=\"$1 <$2>\" \\\n> +\t\t--to=nobody@example.com \\\n> +\t\t--cc-cmd=./cccmd-sed \\\n> +\t\t--suppress-cc=self \\\n> +\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\tsuppress-self-$3.patch && \\\n> +\t\tmv msgtxt1 msgtxt1-$3 && \\\n> +\t\tsed -e '/^$/q' msgtxt1-$3 > msghdr1-$3 && \\\n\nStyle.  No SP between redirection operator and redirection target,\ne.g. >$filename, <$filename, etc.  Some in this patch is done\ncorrectly (e.g. format-patch above), some others are not.\n\n> +\t\trm -f expected-no-cc-$3 && \\\n> +\t\ttouch expected-no-cc-$3 && \\\n\nPlease reserve \"touch\" for \"make sure it has recent timestamp\", not\nfor \"make sure it exists and is empty\".  The above two should be\nmore like:\n\n\t\t>\"expected-no-cc-$3\" &&\n\nAlso, even though it is not required by POSIX, please dquote the\nredirection target filename if you have variable expansion.  Some\nversions of bash (incorrectly) give warning if you don't.\n\n> +\t\tgrep '^Cc:' msghdr1-$3 > actual-no-cc-$3 && \\\n> +\t\ttest_cmp expected-no-cc-$3 actual-no-cc-$3\n> +test suppress-cc.self $3 with name $1 email $2\n> +\n> +$3\n> +\n> +cccmd--\"$1\" <$2>\n> +\n> +Cc: \"$1\" <$2>\n> +Cc: $1 <$2>\n> +Signed-off-by: \"$1\" <$2>\n> +Signed-off-by: $1 <$2>\n> +EOF\n> +}\n> +\n> +test_expect_success $PREREQ 'self name is suppressed' \"\n> +\ttest_suppress_self 'A U Thor' 'author@redhat.com' 'self_name_suppressed'\n> +\"\n> +\n>  test_expect_success $PREREQ 'Show all headers' '\n>  \tgit send-email \\\n>  \t\t--dry-run \\\n"},{"id":"218905","messageId":"20130529214403.GA5956@redhat.com","threadId":"33937","inReplyTo":"7vy5axr0mz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/6] t/send-email.sh: add test for suppress self","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-05-29T21:44:03Z","receivedAt":"2013-05-29T21:44:03Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Wed, May 29, 2013 at 01:28:52PM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> \n> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> > ---\n> \n> Thanks.\n\nThanks, I'll address your comments and repost.\nYou asked some questions below, so I tried to answer.\n\n> >  t/t9001-send-email.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n> >  1 file changed, 41 insertions(+)\n> >\n> > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> > index ebd5c5d..36ecf73 100755\n> > --- a/t/t9001-send-email.sh\n> > +++ b/t/t9001-send-email.sh\n> > @@ -171,6 +171,47 @@ Result: OK\n> >  EOF\n> >  \"\n> >  \n> > +test_suppress_self () {\n> > +\t\ttest_commit $3 &&\n> > +\t\ttest_when_finished \"git reset --hard HEAD^\" &&\n> > +\t\t{\n> > +\t\t\techo \"#!$SHELL_PATH\"\n> > +\t\t\techo sed -n -e s/^cccmd--//p \\\"\\$1\\\"\n> > +\t\t} > cccmd-sed &&\n> > +\t\tchmod +x cccmd-sed &&\n> \n> We can use write_script for this kind of thing, I think.\n\nImportant?\nIt's open-coded elsewhere in this test.\n\n> > +\t\tgit commit --amend --author=\"$1 <$2>\" -F - << EOF && \\\n> \n> Hmm,...  everything below this function is fed as the standard input\n> to \"git commit\" as its updated log message (i.e. \"--amend -F -\")?\n> \n> Puzzled...\n> The EOF I can find is at the very bottom of this function, so there\n> is no \"next command\" that && at the end of the above line is\n> cascading the control to.\n> \n> Doubly puzzled...\n> \n> In any case, please do not add \" \\\" at the end of line when the line\n> ends one command and \"&&\" at the end of line clearly tells the shell\n> that you haven't stopped talking yet.\n\n\nNote that \\ make following lines count as continuation of this one.\nSo they are *not* part of standard input.\n\nLook here:\n\nFOO << EOF && BAR && VAR\nBLABLA\nEOF\n\nthis feeds BLABLA as input to FOO, then\nruns BAR and then VAR.\n\nNow we don't want to put BAR and VAR on same line\nbecause that line is too long.\nSo we write the equivalent: \\ followed by newline\nis same as space for shell, so we can write it as:\n\nFOO << EOF && \\\n BAR && \\\n VAR\nBLABLA\nEOF\n\nClear now?\n\nIf we don't want to use \\, this can also\nbe done like this:\n\nFOO << EOF && \nBLABLA\nEOF\n BAR && \n VAR\n\nI think this is what you suggest.\n\n\n> \n> > +\t\tclean_fake_sendmail && \\\n> > +\t\techo suppress-self-$3.patch > /dev/tty && \\\n> \n> Do we always have /dev/tty?  If this is a leftover debugging, please\n> remove it.\n\nLeftover.\n\n>  If redirecting it to >&2 does not upset what the test\n> does, that is good, too (you can run the test with -v option to view\n> the output).\n> \n> > +\t\tgit format-patch --stdout -1 >suppress-self-$3.patch && \\\n> > +\t\tgit send-email --from=\"$1 <$2>\" \\\n> > +\t\t--to=nobody@example.com \\\n> > +\t\t--cc-cmd=./cccmd-sed \\\n> > +\t\t--suppress-cc=self \\\n> > +\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> > +\t\tsuppress-self-$3.patch && \\\n> > +\t\tmv msgtxt1 msgtxt1-$3 && \\\n> > +\t\tsed -e '/^$/q' msgtxt1-$3 > msghdr1-$3 && \\\n> \n> Style.  No SP between redirection operator and redirection target,\n> e.g. >$filename, <$filename, etc.  Some in this patch is done\n> correctly (e.g. format-patch above), some others are not.\n\nwill fix.\n\n> > +\t\trm -f expected-no-cc-$3 && \\\n> > +\t\ttouch expected-no-cc-$3 && \\\n> \n> Please reserve \"touch\" for \"make sure it has recent timestamp\", not\n> for \"make sure it exists and is empty\".  The above two should be\n> more like:\n> \n> \t\t>\"expected-no-cc-$3\" &&\n\nOK\n\n> Also, even though it is not required by POSIX, please dquote the\n> redirection target filename if you have variable expansion.  Some\n> versions of bash (incorrectly) give warning if you don't.\n\nOK\n\n> > +\t\tgrep '^Cc:' msghdr1-$3 > actual-no-cc-$3 && \\\n> > +\t\ttest_cmp expected-no-cc-$3 actual-no-cc-$3\n> > +test suppress-cc.self $3 with name $1 email $2\n> > +\n> > +$3\n> > +\n> > +cccmd--\"$1\" <$2>\n> > +\n> > +Cc: \"$1\" <$2>\n> > +Cc: $1 <$2>\n> > +Signed-off-by: \"$1\" <$2>\n> > +Signed-off-by: $1 <$2>\n> > +EOF\n> > +}\n> > +\n> > +test_expect_success $PREREQ 'self name is suppressed' \"\n> > +\ttest_suppress_self 'A U Thor' 'author@redhat.com' 'self_name_suppressed'\n> > +\"\n> > +\n> >  test_expect_success $PREREQ 'Show all headers' '\n> >  \tgit send-email \\\n> >  \t\t--dry-run \\\n"},{"id":"218906","messageId":"7vmwrdqu9f.fsf@alter.siamese.dyndns.org","threadId":"33937","inReplyTo":"20130529214403.GA5956@redhat.com","subject":"Re: [PATCH 1/6] t/send-email.sh: add test for suppress self","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-29T22:46:36Z","receivedAt":"2013-05-29T22:46:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" <mst@redhat.com> writes:\n\n>> > +test_suppress_self () {\n>> > +\t\ttest_commit $3 &&\n>> > +\t\ttest_when_finished \"git reset --hard HEAD^\" &&\n>> > +\t\t{\n>> > +\t\t\techo \"#!$SHELL_PATH\"\n>> > +\t\t\techo sed -n -e s/^cccmd--//p \\\"\\$1\\\"\n>> > +\t\t} > cccmd-sed &&\n>> > +\t\tchmod +x cccmd-sed &&\n>> \n>> We can use write_script for this kind of thing, I think.\n>\n> Important?\n> It's open-coded elsewhere in this test.\n\nNot that important, other than that it is nice not to spread what\nthe old tests did before write_script was introduced to new ones, as\nthat is one more thing to update when somebody feels like it.\n\n> If we don't want to use \\, this can also be done like this:\n>\n> FOO << EOF && \n> BLABLA\n> EOF\n>  BAR && \n>  VAR\n>\n> I think this is what you suggest.\n\nYup, that is exactly what I meant (but no leading indentation before\nBAR and VAR).\n\nThat way, it is a lot more clear where the input is (the BLABLA is\nfed to FOO and BAR and VAR do not have anything to do with it).\n\n>> > +\t\tgrep '^Cc:' msghdr1-$3 > actual-no-cc-$3 && \\\n>> > +\t\ttest_cmp expected-no-cc-$3 actual-no-cc-$3\n\nOK, so this is where the message begins, with the commit title \"test\nsupress-cc.self...\".\n\n>> > +test suppress-cc.self $3 with name $1 email $2\n>> > +\n>> > +$3\n>> > +\n>> > +cccmd--\"$1\" <$2>\n>> > +\n>> > +Cc: \"$1\" <$2>\n>> > +Cc: $1 <$2>\n>> > +Signed-off-by: \"$1\" <$2>\n>> > +Signed-off-by: $1 <$2>\n>> > +EOF\n>> > +}\n>> > +\n>> > +test_expect_success $PREREQ 'self name is suppressed' \"\n>> > +\ttest_suppress_self 'A U Thor' 'author@redhat.com' 'self_name_suppressed'\n>> > +\"\n>> > +\n>> >  test_expect_success $PREREQ 'Show all headers' '\n>> >  \tgit send-email \\\n>> >  \t\t--dry-run \\\n"},{"id":"218907","messageId":"7vip21qtnc.fsf@alter.siamese.dyndns.org","threadId":"33937","inReplyTo":"7vmwrdqu9f.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/6] t/send-email.sh: add test for suppress self","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-29T22:59:51Z","receivedAt":"2013-05-29T22:59:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> If we don't want to use \\, this can also be done like this:\n>>\n>> FOO << EOF && \n>> BLABLA\n>> EOF\n>>  BAR && \n>>  VAR\n>>\n>> I think this is what you suggest.\n>\n> Yup, that is exactly what I meant (but no leading indentation before\n> BAR and VAR).\n>\n> That way, it is a lot more clear where the input is (the BLABLA is\n> fed to FOO and BAR and VAR do not have anything to do with it).\n>\n>>> > +\t\tgrep '^Cc:' msghdr1-$3 > actual-no-cc-$3 && \\\n>>> > +\t\ttest_cmp expected-no-cc-$3 actual-no-cc-$3\n>\n> OK, so this is where the message begins, with the commit title \"test\n> supress-cc.self...\".\n\nAnother thing I forgot to say, if you are rerolling this patch\nanyway to follow that style, is that our newer tests typically\nwrite it like this:\n\ntest_supress_self () {\n\ttest_commit $3 &&\n        test_when_finished \"git reset --hard HEAD^\" &&\n        write_script <<-EOF &&\n        sed -n -e s/^cccmd--//p \\\"\\$1\\\"\n\tEOF\n\n\tgit commit --amend --author=\"$1 <$2>\" -F - <<-EOF &&\n\ttest suppress-cc.self $3 with name $1 email $2\n\n\t$3\n\n\tcccmd--\"$1\" <$2>\n\n        Cc: \"$1\" <$2>\n\tCc: $1 <$2>\n\tSigned-off-by: \"$1\" <$2>\n\tSigned-off-by: $1 <$2>\n        EOF\n        \n        clean_fake_sendmail &&\n        git format-patch --stdout -1 >\"suppress-self-$3.patch\" &&\n        git send-email --from=\"$1 <$2>\" \\\n        \t--to=nobody@example.com \\\n                ... other args ...\n        ... verification steps for the send-email output ...\n}\n"},{"id":"218933","messageId":"20130530045154.GA6168@redhat.com","threadId":"33937","inReplyTo":"7vip21qtnc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/6] t/send-email.sh: add test for suppress self","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-05-30T04:51:54Z","receivedAt":"2013-05-30T04:51:54Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Wed, May 29, 2013 at 03:59:51PM -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> >> If we don't want to use \\, this can also be done like this:\n> >>\n> >> FOO << EOF && \n> >> BLABLA\n> >> EOF\n> >>  BAR && \n> >>  VAR\n> >>\n> >> I think this is what you suggest.\n> >\n> > Yup, that is exactly what I meant (but no leading indentation before\n> > BAR and VAR).\n> >\n> > That way, it is a lot more clear where the input is (the BLABLA is\n> > fed to FOO and BAR and VAR do not have anything to do with it).\n> >\n> >>> > +\t\tgrep '^Cc:' msghdr1-$3 > actual-no-cc-$3 && \\\n> >>> > +\t\ttest_cmp expected-no-cc-$3 actual-no-cc-$3\n> >\n> > OK, so this is where the message begins, with the commit title \"test\n> > supress-cc.self...\".\n> \n> Another thing I forgot to say, if you are rerolling this patch\n> anyway to follow that style, is that our newer tests typically\n> write it like this:\n\nWhat exactly should I notice here?\n\n> test_supress_self () {\n> \ttest_commit $3 &&\n>         test_when_finished \"git reset --hard HEAD^\" &&\n>         write_script <<-EOF &&\n>         sed -n -e s/^cccmd--//p \\\"\\$1\\\"\n> \tEOF\n> \n> \tgit commit --amend --author=\"$1 <$2>\" -F - <<-EOF &&\n> \ttest suppress-cc.self $3 with name $1 email $2\n> \n> \t$3\n> \n> \tcccmd--\"$1\" <$2>\n> \n>         Cc: \"$1\" <$2>\n> \tCc: $1 <$2>\n> \tSigned-off-by: \"$1\" <$2>\n> \tSigned-off-by: $1 <$2>\n>         EOF\n>         \n>         clean_fake_sendmail &&\n>         git format-patch --stdout -1 >\"suppress-self-$3.patch\" &&\n>         git send-email --from=\"$1 <$2>\" \\\n>         \t--to=nobody@example.com \\\n>                 ... other args ...\n>         ... verification steps for the send-email output ...\n> }\n"}]}