[PATCH] git-send-email: Delete additional From message body

[PATCH] git-send-email: Delete additional From message body

[brilliantov]

Additional From added to message body if git-send-email run
with --from parameters

Signed-off-by: Brilliantov Kirill Vladimirovich <brilliantov@inbox.ru>
---
 git-send-email.perl | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index b660cc2..92ec74a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1522,24 +1522,21 @@ foreach my $t (@files) {
 		$subject = quote_subject($subject, $auto_8bit_encoding);
 	}
 
-	if (defined $sauthor and $sauthor ne $sender) {
-		$message = "From: $author\n\n$message";
-		if (defined $author_encoding) {
-			if ($has_content_type) {
-				if ($body_encoding eq $author_encoding) {
-					# ok, we already have the right encoding
-				}
-				else {
-					# uh oh, we should re-encode
-				}
+	if (defined $author_encoding) {
+		if ($has_content_type) {
+			if ($body_encoding eq $author_encoding) {
+				# ok, we already have the right encoding
 			}
 			else {
-				$xfer_encoding = '8bit' if not defined $xfer_encoding;
-				$has_content_type = 1;
-				push @xh,
-				  "Content-Type: text/plain; charset=$author_encoding";
+				# uh oh, we should re-encode
 			}
 		}
+		else {
+			$xfer_encoding = '8bit' if not defined $xfer_encoding;
+			$has_content_type = 1;
+			push @xh,
+			  "Content-Type: text/plain; charset=$author_encoding";
+		}
 	}
 	if (defined $target_xfer_encoding) {
 		$xfer_encoding = '8bit' if not defined $xfer_encoding;
-- 
2.1.4

Re: [PATCH] git-send-email: Delete additional From message body

[Junio C Hamano]

brilliantov@inbox.ru writes:

> Additional From added to message body if git-send-email run
> with --from parameters
>
> Signed-off-by: Brilliantov Kirill Vladimirovich <brilliantov@inbox.ru>
> ---
>  git-send-email.perl | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index b660cc2..92ec74a 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1522,24 +1522,21 @@ foreach my $t (@files) {
>  		$subject = quote_subject($subject, $auto_8bit_encoding);
>  	}
>  
> -	if (defined $sauthor and $sauthor ne $sender) {
> -		$message = "From: $author\n\n$message";

This condition attempts to say "Look at the author taken from the
From: line of the message, and if that is different from the sender
(which will be recorded in the header of the resulting e-mail), then
we do need to add an additional in-body From: line to record the
author of the commit, who may be different from the name that would
appear in the e-mail."

In other words, the "Additional From" is very much deliberate when
the "--from" option is used and is different from the patch author.
And I do not see this "add in-body header if the author and the
sender are different" you are removing from here appear anywhere in
the added text of your patch.  So one consequence of this patch is
to _never_ use the in-body From: line.

That needs a serious justification.

Now, it is entirely possible that what you wanted to do (and what
you are doing) in this patch may be different from "unconditionally,
remove the in-body From: line".  Perhaps the code mistakenly
declares that the sender given via "--from" and the author are
different, possibly due to a bug in rfc2047 unquoting code or
something, and you are only trying to make sure that the code
notices they are really the same.  But I somehow have a feeling that
that is not what is going on, because in such a patch, there still
will be some code that adds "From: $author" at the very beginning of
outgoing message body; such a fix might update the "if ()" condition
used to guard it (e.g. $sauthor and $sender may have to get further
cleansed before comparing), but I do not see anything of that sort,
either.  Also all the changes below do not have anything to do with
the additional in-body "From: " line.

So, I dunno.  I cannot tell what you are trying to do, and the only
immediate effect of the change I can read is that in-body "From:"
line is unconditionally disabled, which is certainly not something
we would want to do.

Puzzled...


> -		if (defined $author_encoding) {
> -			if ($has_content_type) {
> -				if ($body_encoding eq $author_encoding) {
> -					# ok, we already have the right encoding
> -				}
> -				else {
> -					# uh oh, we should re-encode
> -				}
> +	if (defined $author_encoding) {
> +		if ($has_content_type) {
> +			if ($body_encoding eq $author_encoding) {
> +				# ok, we already have the right encoding
>  			}
>  			else {
> -				$xfer_encoding = '8bit' if not defined $xfer_encoding;
> -				$has_content_type = 1;
> -				push @xh,
> -				  "Content-Type: text/plain; charset=$author_encoding";
> +				# uh oh, we should re-encode
>  			}
>  		}
> +		else {
> +			$xfer_encoding = '8bit' if not defined $xfer_encoding;
> +			$has_content_type = 1;
> +			push @xh,
> +			  "Content-Type: text/plain; charset=$author_encoding";
> +		}
>  	}
>  	if (defined $target_xfer_encoding) {
>  		$xfer_encoding = '8bit' if not defined $xfer_encoding;