[PATCH] git-send-email: show all headers when sending mail

[PATCH] git-send-email: show all headers when sending mail

[David D. Kilzer]

As a git newbie, it was confusing to set an In-Reply-To header but then
not see it printed when the git-send-email command was run.

This patch prints all headers that would be sent to sendmail or an SMTP
server instead of only printing From, Subject, Cc, To.  It also removes
the now-extraneous Date header after the "Log says" line.

Added test to t/t9001-send-email.sh.

Signed-off-by: David D. Kilzer <ddkilzer@kilzer.net>
---

I'd like to see this applied to the maint branch.

 git-send-email.perl   |    4 ++--
 t/t9001-send-email.sh |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 8760cf8..e3ea786 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -573,7 +573,7 @@ X-Mailer: git-send-email $gitversion
 	if ($quiet) {
 		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
 	} else {
-		print (($dry_run ? "Dry-" : "")."OK. Log says:\nDate: $date\n");
+		print (($dry_run ? "Dry-" : "")."OK. Log says:\n");
 		if ($smtp_server !~ m#^/#) {
 			print "Server: $smtp_server\n";
 			print "MAIL FROM:<$raw_from>\n";
@@ -581,7 +581,7 @@ X-Mailer: git-send-email $gitversion
 		} else {
 			print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
 		}
-		print "From: $sanitized_sender\nSubject: $subject\nCc: $cc\nTo: $to\n\n";
+		print $header, "\n";
 		if ($smtp) {
 			print "Result: ", $smtp->code, ' ',
 				($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 83f9470..fc03e40 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -41,4 +41,42 @@ test_expect_success \
     'Verify commandline' \
     'diff commandline expected'
 
+cat >expected-show-all-headers <<\EOF
+0001-Second.patch
+(mbox) Adding cc: A <author@example.com> from line 'From: A <author@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>
+From: Example <from@example.com>
+To: to@example.com
+Cc: cc@example.com, A <author@example.com>
+Subject: [PATCH 1/1] Second.
+Date: Mon, 12 Nov 2007 15:01:19 +0000
+Message-Id: <1194879679-15720-1-git-send-email-from@example.com>
+X-Mailer: git-send-email 1.5.3.5.38.gb33cf-dirty
+In-Reply-To: <unique-message-id@example.com>
+References: <unique-message-id@example.com>
+
+Result: OK
+EOF
+
+replace_header () {
+	EXPECTED=expected-show-all-headers &&
+	ACTUAL=actual-show-all-headers &&
+	REPLACEMENT=`cat ${ACTUAL} | grep "^$1:"` &&
+	if [ ! -z "${REPLACEMENT}" ]; then \
+		cat ${EXPECTED} | sed -e "s/^$1: .*\$/${REPLACEMENT}/" > ${EXPECTED}.$$ && \
+		mv -f ${EXPECTED}.$$ ${EXPECTED}
+	fi
+}
+
+test_expect_success 'Show all headers' '
+	git send-email --dry-run --from="Example <from@example.com>" --to=to@example.com --cc=cc@example.com --bcc=bcc@example.com --in-reply-to="<unique-message-id@example.com>" --smtp-server relay.example.com $patches > actual-show-all-headers &&
+	replace_header "Date" &&
+	replace_header "Message-Id" &&
+	replace_header "X-Mailer" &&
+	diff -u expected-show-all-headers actual-show-all-headers
+'
+
 test_done
-- 
1.5.3.4

Re: [PATCH] git-send-email: show all headers when sending mail

[Junio C Hamano]

"David D. Kilzer" <ddkilzer@kilzer.net> writes:

> +replace_header () {
> +	EXPECTED=expected-show-all-headers &&
> +	ACTUAL=actual-show-all-headers &&
> +	REPLACEMENT=`cat ${ACTUAL} | grep "^$1:"` &&
> +	if [ ! -z "${REPLACEMENT}" ]; then \
> +		cat ${EXPECTED} | sed -e "s/^$1: .*\$/${REPLACEMENT}/" > ${EXPECTED}.$$ && \
> +		mv -f ${EXPECTED}.$$ ${EXPECTED}
> +	fi
> +}

If the actual output did not have an asked-for field,
REPLACEMENT will be empty and the breakage will go unnoticed,
won't it?

It would probably be better to write it this way:

	test_expect_success 'Show all headers' '

		git send-email \
                	--dry-run \
                        --from="Example <from@example.com>" \
                        --to=to@example.com \
                        --cc=cc@example.com \
                        --bcc=bcc@example.com \
                        --in-reply-to="<unique-message-id@example.com>" \
                        --smtp-sever relay.example.com \
                        $patches |
		sed	-e "s/^\(Date:\).*/1 DATE-STRING/" \
                	-e "s/^\(Message-Id:\).*/1 ID-STRING/" \
                        -e "s/^\(X-Mailer:\).*/1 X-MAILER-STRING/" \
			>actual &&
		diff -u expected actual

        '

and prepare the expected output with the varying field already
replaced with the placeholder string.

Oh, by the way, do not cat a single file and pipe it to another
command.  There may still be a few such stupidity in our test
scripts but let's not add even more of them...

[PATCH v2] git-send-email: show all headers when sending mail

[David D. Kilzer]

As a git newbie, it was confusing to set an In-Reply-To header but then
not see it printed when the git-send-email command was run.

This patch prints all headers that would be sent to sendmail or an SMTP
server instead of only printing From, Subject, Cc, To.  It also removes
the now-extraneous Date header after the "Log says" line.

Added test to t/t9001-send-email.sh.

Signed-off-by: David D. Kilzer <ddkilzer@kilzer.net>
---

Updated t/t9001-send-email.sh per feedback from Junio C Hamano.

 git-send-email.perl   |    4 ++--
 t/t9001-send-email.sh |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 2b1f1b5..f4539a0 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -575,7 +575,7 @@ X-Mailer: git-send-email $gitversion
 	if ($quiet) {
 		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
 	} else {
-		print (($dry_run ? "Dry-" : "")."OK. Log says:\nDate: $date\n");
+		print (($dry_run ? "Dry-" : "")."OK. Log says:\n");
 		if ($smtp_server !~ m#^/#) {
 			print "Server: $smtp_server\n";
 			print "MAIL FROM:<$raw_from>\n";
@@ -583,7 +583,7 @@ X-Mailer: git-send-email $gitversion
 		} else {
 			print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
 		}
-		print "From: $sanitized_sender\nSubject: $subject\nCc: $cc\nTo: $to\n\n";
+		print $header, "\n";
 		if ($smtp) {
 			print "Result: ", $smtp->code, ' ',
 				($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 83f9470..659f9c7 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -41,4 +41,41 @@ test_expect_success \
     'Verify commandline' \
     'diff commandline expected'
 
+cat >expected-show-all-headers <<\EOF
+0001-Second.patch
+(mbox) Adding cc: A <author@example.com> from line 'From: A <author@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>
+From: Example <from@example.com>
+To: to@example.com
+Cc: cc@example.com, A <author@example.com>
+Subject: [PATCH 1/1] Second.
+Date: DATE-STRING
+Message-Id: MESSAGE-ID-STRING
+X-Mailer: X-MAILER-STRING
+In-Reply-To: <unique-message-id@example.com>
+References: <unique-message-id@example.com>
+
+Result: OK
+EOF
+
+test_expect_success 'Show all headers' '
+	git send-email \
+		--dry-run \
+		--from="Example <from@example.com>" \
+		--to=to@example.com \
+		--cc=cc@example.com \
+		--bcc=bcc@example.com \
+		--in-reply-to="<unique-message-id@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 &&
+	diff -u expected-show-all-headers actual-show-all-headers
+'
+
 test_done
-- 
1.5.3.4

Re: [PATCH v2] git-send-email: show all headers when sending mail

[Junio C Hamano]

Thanks.  Looks nice and obviously correct.

One thing that has been bugging me for a long time now stands
out like a sore thumb much more: empty Cc: is shown.

    $ git-send-email --dry-run --to=junio@my.isp.net 0001-branch-contains.txt
    Who should the emails appear to be from? [Junio C Hamano <gitster@pobox.com>]
    Emails will be sent from: Junio C Hamano <gitster@pobox.com>
    Message-ID to be used as In-Reply-To for the first email?
    0001-branch-contains.txt
    Dry-OK. Log says:
    Date: Mon, 19 Nov 2007 00:10:04 -0800
    Server: my.isp.net
    MAIL FROM:<gitster@pobox.com>
    RCPT TO:<junio@my.isp.net>
    From: Junio C Hamano <gitster@pobox.com>
    Subject: [PATCH] branch --contains=<commit>
    Cc:
    To: junkio@cox.net

    Result: OK

[PATCH] Don't print an empty Cc header in SMTP mode when there's no cc recipient defined

[Ask Bjørn Hansen]

Signed-off-by: Ask Bjørn Hansen <ask@develooper.com>
---

There's some duplicate code between "what we do for sendmail"
and "what we do for SMTP" paths that should be fixed - this doesn't
do that, it only makes the SMTP path skip empty Cc lines...

 git-send-email.perl |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index fd0a4ad..65620ab 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -651,7 +651,11 @@ X-Mailer: git-send-email $gitversion
 		} else {
 			print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
 		}
-		print "From: $sanitized_sender\nSubject: $subject\nCc: $cc\nTo: $to\n\n";
+		print "From: $sanitized_sender\n"
+		     . "Subject: $subject\n"
+		     . ($cc ? "Cc: $cc\n" : "")
+		     . "To: $to\n"
+		     . "\n";
 		if ($smtp) {
 			print "Result: ", $smtp->code, ' ',
 				($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
-- 
1.5.3.5.561.g140d

Re: [PATCH v2] git-send-email: show all headers when sending mail

[David D. Kilzer]

I can't seem to reproduce this.  Could you send me (off-list)
0001-branch-contains.txt and any relevant config bits?

Dave


Junio C Hamano <gitster@pobox.com> wrote:

> Thanks.  Looks nice and obviously correct.
> 
> One thing that has been bugging me for a long time now stands
> out like a sore thumb much more: empty Cc: is shown.
> 
>     $ git-send-email --dry-run --to=junio@my.isp.net 0001-branch-contains.txt
>     Who should the emails appear to be from? [Junio C Hamano
> <gitster@pobox.com>]
>     Emails will be sent from: Junio C Hamano <gitster@pobox.com>
>     Message-ID to be used as In-Reply-To for the first email?
>     0001-branch-contains.txt
>     Dry-OK. Log says:
>     Date: Mon, 19 Nov 2007 00:10:04 -0800
>     Server: my.isp.net
>     MAIL FROM:<gitster@pobox.com>
>     RCPT TO:<junio@my.isp.net>
>     From: Junio C Hamano <gitster@pobox.com>
>     Subject: [PATCH] branch --contains=<commit>
>     Cc:
>     To: junkio@cox.net
> 
>     Result: OK

Re: [PATCH v2] git-send-email: show all headers when sending mail

[Ask Bjørn Hansen]

On Nov 19, 2007, at 10:50, David D. Kilzer wrote:

> Junio C Hamano <gitster@pobox.com> wrote:
>
>> Thanks.  Looks nice and obviously correct.
>>
>> One thing that has been bugging me for a long time now stands
>> out like a sore thumb much more: empty Cc: is shown.
>
> I can't seem to reproduce this.  Could you send me (off-list)
> 0001-branch-contains.txt and any relevant config bits?

You need to make git-send-email not have an implicit Cc (from signed- 
off-by or some such), then it'll appear.

I sent a patch yesterday for it,

Subject: [PATCH] Don't print an empty Cc header in SMTP mode when  
there's no cc recipient defined
Message-Id: <1195469295-5774-1-git-send-email-ask@develooper.com>



  - ask

-- 
http://develooper.com/ - http://askask.com/