From: NeilBrown <neilb@ownmail.net>
Date: Wed, 17 Sep 2025 06:21:01 GMT
Subject: Re: [PATCH] send-email: don't duplicate Reply-to:
Message-ID: <175809006105.1696783.12105140613644905214@noble.neil.brown.name>
In-Reply-To: <xmqqbjnbci6j.fsf@gitster.g>

On Tue, 16 Sep 2025, Junio C Hamano wrote:
> Thanks for sending a patch.  

and thanks for the review.
It all makes - I've made some changes and will resend.

NeilBrown


> 
> NeilBrown <neilb@ownmail.net> writes:
> 
> > If I run
> >   git send-email --compose --reply-to 'ME <my@address.net>' .....
> >
> > and edit the intro message, then the message will get two copies of the
> > Reply-To field.  gmail.com rejects such messages.
> 
> A commit log message that begins with such a clearly written problem
> description is always welcome.
> 
> > This happens because a Reply-To is inserted into the intro message
> > template, and then the intro message headers are copied and another
> > Reply-to is added.
> 
> OK.
> 
> It took me a few minutes to follow the code based on the above three
> lines to figure out exactly what is going on.  The key realization I
> needed was that the if/elsif/... chain being touched is sifting the
> e-mail headers that appear in the message being sent into two kinds,
> ones whose values are understood and parsed into individual variables
> that have their own meaning (like $reply_to), and others that are
> not understood by the code and thrown into @xh array to be blindly
> replayed into the resulting message later.
> 
> By parsing the "Reply-To:" header into $reply_to, the code with your
> patch stops throwing it in @xh and that is how duplicate headers are
> prevented.
> 
> > This patch fixes the problem by noticing the Reply-To: header when the
> > intro is parsed, and using it to assign $reply_to rather than blindly
> > coping it into the new headers.
> 
> And what I thought missing is mentioned as part of the solution
> here.  I would have preferred to see some more described in the
> second paragraph that analyses how the breakage happens---it would
> have saved me a few minutes ;-).
> 
> A few issues in the log message.
> 
>  * The commit, when prosessed by "git am", will record your name as
>    "NeilBrown <neilb@ownmail.net>" because that is the name and
>    address used on the "From:" header of the e-mail I am responding
>    to.  It does not match the name/address used for sign-off below,
>    which is not right.  If <neil@brown.name> is the address you want
>    to be known as to this project, you'd need to override the author
>    name by inserting a line "From: NeilBrown <neil@brown.name>" at
>    the very beginning of the e-mail body, plus a blank line to
>    separate it from the body of the message.
> 
>  * We prefer to see the solution described as if you are giving an
>    order to somebody sitting on the keyboard to "make the code look
>    like so", e.g.
> 
>      Fix the problem by parsing Reply-To: header into $reply_to
>      variable, to be reproduced on a single header line later,
>      instead of getting passed unrecognised and left in @xh to be
>      copied into the outgoing message.
> 
>    or something like that.
> 
> > Signed-off-by: NeilBrown <neil@brown.name>
> > ---
> >  git-send-email.perl | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> Thanks.
> 
> >
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > index 437f8ac46a85..e2248c223119 100755
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -1931,6 +1931,9 @@ sub pre_process_file {
> >  					$in_reply_to = $1;
> >  				}
> >  			}
> > +			elsif (/^Reply-To: (.*)/i) {
> > +				$reply_to = $1;
> > +			}
> >  			elsif (/^References: (.*)/i) {
> >  				if (!$initial_in_reply_to || $thread) {
> >  					$references = $1;
>