threads / patch / 10833

patchgit-send-email: show all headers when sending mail

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

## tl;dr

7 messages between Nov 12, 2007 and Nov 20, 2007. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

David D. Kilzer· Nov 12, 2007, 16:01 UTC · lore

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(-)
Show changes to 2 files +40 −2

git-send-email.perl, t/t9001-send-email.sh

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
Junio C Hamano· Nov 13, 2007, 07:28 UTC · re: David D. Kilzer · lore

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

"David D. Kilzer" <ddkilzer@kilzer.net> writes:
Show 9 quoted lines
> +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...

David D. Kilzer· Nov 19, 2007, 04:14 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to 2 files +39 −2

git-send-email.perl, t/t9001-send-email.sh

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
Junio C Hamano· Nov 19, 2007, 08:17 UTC · re: David D. Kilzer · lore

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

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
Ask Bjørn Hansen· Nov 19, 2007, 10:48 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to git-send-email.perl +5 −1
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
David D. Kilzer· Nov 19, 2007, 18:50 UTC · re: Junio C Hamano · lore

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

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:
Show 22 quoted lines
> 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
Ask Bjørn Hansen· Nov 20, 2007, 01:53 UTC · re: David D. Kilzer · lore

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

On Nov 19, 2007, at 10:50, David D. Kilzer wrote:
Show 9 quoted lines
> 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/

← back to recent threads