git-send-email: Support for Reply-To

git-send-email: Support for Reply-To

[Christian Ludwig]

Hi,

this is the third iteration of this series. There was a request to
rebase the changes on the refactoring patch b6049542 ("send-email:
extract email-parsing code into a subroutine", 2017-12-15). This is
the result.

The diffstat is the same compared to the last revision. It could be
made smaller by sacrificing readibility and breaking the scheme
introduced by the refactoring patch. But I do agree that send-email's
readability could benefit from slicing it into handy functions. And the
refactoring patch reduces the nesting of loops/conditionals.

But it's your code, you decide. I can re-send a fixed-up v2 without the
rebasing.


So long,


 - Christian

[PATCH v3 1/2] send-email: Rename variable for clarity

[Christian Ludwig]

The SMTP protocol has both, the 'Reply-To' and the 'In-Reply-To' header
fields. We only use the latter. To avoid confusion, rename the variable
for it.

Signed-off-by: Christian Ludwig <chrissicool@gmail.com>
---
 git-send-email.perl | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d0dcc6d7f..9eb12b5ba 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -167,13 +167,13 @@ my $re_encoded_word = qr/=\?($re_token)\?($re_token)\?($re_encoded_text)\?=/;
 
 # Variables we fill in automatically, or via prompting:
 my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,
-	$initial_reply_to,$initial_subject,@files,
+	$initial_in_reply_to,$initial_subject,@files,
 	$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);
 
 my $envelope_sender;
 
 # Example reply to:
-#$initial_reply_to = ''; #<20050203173208.GA23964@foobar.com>';
+#$initial_in_reply_to = ''; #<20050203173208.GA23964@foobar.com>';
 
 my $repo = eval { Git->repository() };
 my @repo = $repo ? ($repo) : ();
@@ -315,7 +315,7 @@ die __("--dump-aliases incompatible with other options\n")
     if !$help and $dump_aliases and @ARGV;
 $rc = GetOptions(
 		    "sender|from=s" => \$sender,
-                    "in-reply-to=s" => \$initial_reply_to,
+                    "in-reply-to=s" => \$initial_in_reply_to,
 		    "subject=s" => \$initial_subject,
 		    "to=s" => \@initial_to,
 		    "to-cmd=s" => \$to_cmd,
@@ -677,7 +677,7 @@ if ($compose) {
 
 	my $tpl_sender = $sender || $repoauthor || $repocommitter || '';
 	my $tpl_subject = $initial_subject || '';
-	my $tpl_reply_to = $initial_reply_to || '';
+	my $tpl_in_reply_to = $initial_in_reply_to || '';
 
 	print $c <<EOT1, Git::prefix_lines("GIT: ", __ <<EOT2), <<EOT3;
 From $tpl_sender # This line is ignored.
@@ -690,7 +690,7 @@ Clear the body content if you don't wish to send a summary.
 EOT2
 From: $tpl_sender
 Subject: $tpl_subject
-In-Reply-To: $tpl_reply_to
+In-Reply-To: $tpl_in_reply_to
 
 EOT3
 	for my $f (@files) {
@@ -729,7 +729,7 @@ EOT3
 		$sender = delete($parsed_email{'From'});
 	}
 	if ($parsed_email{'In-Reply-To'}) {
-		$initial_reply_to = delete($parsed_email{'In-Reply-To'});
+		$initial_in_reply_to = delete($parsed_email{'In-Reply-To'});
 	}
 	if ($parsed_email{'Subject'}) {
 		$initial_subject = delete($parsed_email{'Subject'});
@@ -912,16 +912,16 @@ sub expand_one_alias {
 @initial_cc = process_address_list(@initial_cc);
 @bcclist = process_address_list(@bcclist);
 
-if ($thread && !defined $initial_reply_to && $prompting) {
-	$initial_reply_to = ask(
+if ($thread && !defined $initial_in_reply_to && $prompting) {
+	$initial_in_reply_to = ask(
 		__("Message-ID to be used as In-Reply-To for the first email (if any)? "),
 		default => "",
 		valid_re => qr/\@.*\./, confirm_only => 1);
 }
-if (defined $initial_reply_to) {
-	$initial_reply_to =~ s/^\s*<?//;
-	$initial_reply_to =~ s/>?\s*$//;
-	$initial_reply_to = "<$initial_reply_to>" if $initial_reply_to ne '';
+if (defined $initial_in_reply_to) {
+	$initial_in_reply_to =~ s/^\s*<?//;
+	$initial_in_reply_to =~ s/>?\s*$//;
+	$initial_in_reply_to = "<$initial_in_reply_to>" if $initial_in_reply_to ne '';
 }
 
 if (!defined $smtp_server) {
@@ -941,7 +941,7 @@ if ($compose && $compose > 0) {
 }
 
 # Variables we set as part of the loop over files
-our ($message_id, %mail, $subject, $reply_to, $references, $message,
+our ($message_id, %mail, $subject, $in_reply_to, $references, $message,
 	$needs_confirm, $message_num, $ask_default);
 
 sub extract_valid_address {
@@ -1350,9 +1350,9 @@ Message-Id: $message_id
 	if ($use_xmailer) {
 		$header .= "X-Mailer: git-send-email $gitversion\n";
 	}
-	if ($reply_to) {
+	if ($in_reply_to) {
 
-		$header .= "In-Reply-To: $reply_to\n";
+		$header .= "In-Reply-To: $in_reply_to\n";
 		$header .= "References: $references\n";
 	}
 	if (@xh) {
@@ -1529,8 +1529,8 @@ EOF
 	return 1;
 }
 
-$reply_to = $initial_reply_to;
-$references = $initial_reply_to || '';
+$in_reply_to = $initial_in_reply_to;
+$references = $initial_in_reply_to || '';
 $subject = $initial_subject;
 $message_num = 0;
 
@@ -1740,9 +1740,9 @@ foreach my $t (@files) {
 
 	# set up for the next message
 	if ($thread && $message_was_sent &&
-		($chain_reply_to || !defined $reply_to || length($reply_to) == 0 ||
+		($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||
 		$message_num == 1)) {
-		$reply_to = $message_id;
+		$in_reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
 		} else {
-- 
2.16.2

[PATCH v3 2/2] send-email: Support separate Reply-To address

[Christian Ludwig]

In some projects contributions from groups are only accepted from a
common group email address. But every individual may want to receive
replies to her own personal address. That's what we have 'Reply-To'
headers for in SMTP. So introduce an optional '--reply-to' command
line option.

This patch re-uses the $reply_to variable. This could break
out-of-tree patches!

Signed-off-by: Christian Ludwig <chrissicool@gmail.com>
---
 Documentation/git-send-email.txt       |  5 +++++
 contrib/completion/git-completion.bash |  2 +-
 git-send-email.perl                    | 18 +++++++++++++++++-
 t/t9001-send-email.sh                  |  2 ++
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 8060ea35c..71ef97ba9 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -84,6 +84,11 @@ See the CONFIGURATION section for `sendemail.multiEdit`.
 	the value of GIT_AUTHOR_IDENT, or GIT_COMMITTER_IDENT if that is not
 	set, as returned by "git var -l".
 
+--reply-to=<address>::
+	Specify the address where replies from recipients should go to.
+	Use this if replies to messages should go to another address than what
+	is specified with the --from parameter.
+
 --in-reply-to=<identifier>::
 	Make the first mail (or all the mails with `--no-thread`) appear as a
 	reply to the given Message-Id, which avoids breaking threads to
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c7d5c7af2..4805b92ba 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2081,7 +2081,7 @@ _git_send_email ()
 			--compose --confirm= --dry-run --envelope-sender
 			--from --identity
 			--in-reply-to --no-chain-reply-to --no-signed-off-by-cc
-			--no-suppress-from --no-thread --quiet
+			--no-suppress-from --no-thread --quiet --reply-to
 			--signed-off-by-cc --smtp-pass --smtp-server
 			--smtp-server-port --smtp-encryption= --smtp-user
 			--subject --suppress-cc= --suppress-from --thread --to
diff --git a/git-send-email.perl b/git-send-email.perl
index 9eb12b5ba..707ec9eb7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -57,6 +57,7 @@ git send-email --dump-aliases
     --[no-]cc               <str>  * Email Cc:
     --[no-]bcc              <str>  * Email Bcc:
     --subject               <str>  * Email "Subject:"
+    --reply-to              <str>  * Email "Reply-To:"
     --in-reply-to           <str>  * Email "In-Reply-To:"
     --[no-]xmailer                 * Add "X-Mailer:" header (default).
     --[no-]annotate                * Review each patch that will be sent in an editor.
@@ -167,7 +168,7 @@ my $re_encoded_word = qr/=\?($re_token)\?($re_token)\?($re_encoded_text)\?=/;
 
 # Variables we fill in automatically, or via prompting:
 my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,
-	$initial_in_reply_to,$initial_subject,@files,
+	$initial_in_reply_to,$reply_to,$initial_subject,@files,
 	$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);
 
 my $envelope_sender;
@@ -316,6 +317,7 @@ die __("--dump-aliases incompatible with other options\n")
 $rc = GetOptions(
 		    "sender|from=s" => \$sender,
                     "in-reply-to=s" => \$initial_in_reply_to,
+		    "reply-to=s" => \$reply_to,
 		    "subject=s" => \$initial_subject,
 		    "to=s" => \@initial_to,
 		    "to-cmd=s" => \$to_cmd,
@@ -678,6 +680,7 @@ if ($compose) {
 	my $tpl_sender = $sender || $repoauthor || $repocommitter || '';
 	my $tpl_subject = $initial_subject || '';
 	my $tpl_in_reply_to = $initial_in_reply_to || '';
+	my $tpl_reply_to = $reply_to || '';
 
 	print $c <<EOT1, Git::prefix_lines("GIT: ", __ <<EOT2), <<EOT3;
 From $tpl_sender # This line is ignored.
@@ -689,6 +692,7 @@ for the patch you are writing.
 Clear the body content if you don't wish to send a summary.
 EOT2
 From: $tpl_sender
+Reply-To: $tpl_reply_to
 Subject: $tpl_subject
 In-Reply-To: $tpl_in_reply_to
 
@@ -731,6 +735,9 @@ EOT3
 	if ($parsed_email{'In-Reply-To'}) {
 		$initial_in_reply_to = delete($parsed_email{'In-Reply-To'});
 	}
+	if ($parsed_email{'Reply-To'}) {
+		$reply_to = delete($parsed_email{'Reply-To'});
+	}
 	if ($parsed_email{'Subject'}) {
 		$initial_subject = delete($parsed_email{'Subject'});
 		print $c2 "Subject: " .
@@ -924,6 +931,12 @@ if (defined $initial_in_reply_to) {
 	$initial_in_reply_to = "<$initial_in_reply_to>" if $initial_in_reply_to ne '';
 }
 
+if (defined $reply_to) {
+	$reply_to =~ s/^\s+|\s+$//g;
+	($reply_to) = expand_aliases($reply_to);
+	$reply_to = sanitize_address($reply_to);
+}
+
 if (!defined $smtp_server) {
 	my @sendmail_paths = qw( /usr/sbin/sendmail /usr/lib/sendmail );
 	push @sendmail_paths, map {"$_/sendmail"} split /:/, $ENV{PATH};
@@ -1355,6 +1368,9 @@ Message-Id: $message_id
 		$header .= "In-Reply-To: $in_reply_to\n";
 		$header .= "References: $references\n";
 	}
+	if ($reply_to) {
+		$header .= "Reply-To: $reply_to\n";
+	}
 	if (@xh) {
 		$header .= join("\n", @xh) . "\n";
 	}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 19601fb54..e80eacbb1 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -224,6 +224,7 @@ Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 In-Reply-To: <unique-message-id@example.com>
 References: <unique-message-id@example.com>
+Reply-To: Reply <reply@example.com>
 
 Result: OK
 EOF
@@ -316,6 +317,7 @@ test_expect_success $PREREQ 'Show all headers' '
 		--dry-run \
 		--suppress-cc=sob \
 		--from="Example <from@example.com>" \
+		--reply-to="Reply <reply@example.com>" \
 		--to=to@example.com \
 		--cc=cc@example.com \
 		--bcc=bcc@example.com \
-- 
2.16.2

Re: git-send-email: Support for Reply-To

[Junio C Hamano]

Christian Ludwig <chrissicool@gmail.com> writes:

> this is the third iteration of this series. There was a request to
> rebase the changes on the refactoring patch b6049542 ("send-email:
> extract email-parsing code into a subroutine", 2017-12-15). This is
> the result.

Thanks.  Let me Cc the party who did the refactoring, as it was
unclear how much value the change that did only refactoring without
having an actual user of the end result---now we do have code that
uses it.

> The diffstat is the same compared to the last revision. It could be
> made smaller by sacrificing readibility and breaking the scheme
> introduced by the refactoring patch. But I do agree that send-email's
> readability could benefit from slicing it into handy functions. And the
> refactoring patch reduces the nesting of loops/conditionals.

Thanks.

I compared the result of applying these on top of the refactoring
commit, and cherry-picking the previous round on top of the same
refactoring commit, and found that they pretty much result in the
same code (which was an exercise for me to gain confidence in this
code).