All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rafael Aquini <aquini@redhat.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] send-email: restore --in-reply-to superseding behavior
Date: Thu, 25 Jun 2020 14:47:15 -0400	[thread overview]
Message-ID: <20200625184715.GC2117795@optiplex-lnx> (raw)
In-Reply-To: <20200624234539.GH1987277@optiplex-lnx>

On Wed, Jun 24, 2020 at 07:45:43PM -0400, Rafael Aquini wrote:
> On Wed, Jun 24, 2020 at 02:33:14PM -0700, Junio C Hamano wrote:
> > Rafael Aquini <aquini@redhat.com> writes:
> > 
> > > git send-email --in-reply-to= fails to override the email headers,
> > > if they're present in the output of format-patch, which breakes the
> > 
> > Will do s/breakes/breaks/ while applying.
> >
> 
> UGH! I've been fat-fingering typos the whole day, today... Sorry about
> that one.
> 
>  
> > It makes me wonder, however, why it is a good idea to have the I-R-T
> > in the format patch output in the first place.
> > 
> > >  			elsif (/^In-Reply-To: (.*)/i) {
> > > -				$in_reply_to = $1;
> > > +				if (!$initial_in_reply_to) {
> > > +					$in_reply_to = $1;
> > > +				}
> > 
> > I can see how this would work the way it should for the first
> > message we send out, so it would work well for a single patch.
> > 
> > But what does this change do to the chaining (either making [PATCH
> > 1/N] thru [PATCH N/N] as responses to the cover letter [PATCH 0/N],
> > or making [PATCH n+1/N] as response to [PATCH n/N] for 1 <= n < N)
> > of multiple messages?
> > 
> > When you prepare a series whose 1..N/N are all pointing at 0/N with
> > the already prepared In-Reply-To (so you have N+1 files to send
> > out), wouldn't you want to make 0/N a reply to a particular message
> > you specify on the command line, while keeping the relationship
> > among your messages intact?  Doesn't having $initial_in_reply_to
> > (i.e. command line override) help above code break the chaning?
> >
> 
> This change will make all emails to appear as a reply to the msgid
> fed to --in-reply-to. I see your point, though, and at its light 
> I think now this patch, is actually incomplete. 
> 
> With this change we get back the override desired behavior,
> but it also breaks the contract, according to the man page.
> 
> "
>  --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 provide a new patch series. The second and subsequent emails will be sent as
>      replies according to the --[no-]chain-reply-to setting.
> "
> 
> I drove the change based on my usecase, which is marginal to the
> multi-part reply case. 
> 
> I guess we just need the following, for a complete solution:
> 
> 
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index dc95656f75..768296ea0a 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1699,10 +1699,14 @@ sub process_file {
>  				$xfer_encoding = $1 if not defined $xfer_encoding;
>  			}
>  			elsif (/^In-Reply-To: (.*)/i) {
> -				$in_reply_to = $1;
> +				if (!$initial_in_reply_to || $thread) {
> +					$in_reply_to = $1;
> +				}
>  			}
>  			elsif (/^References: (.*)/i) {
> -				$references = $1;
> +				if (!$initial_in_reply_to || $thread) {
> +					$references = $1;
> +				}
>  			}
>  			elsif (!/^Date:\s/i && /^[-A-Za-z]+:\s+\S/) {
>  				push @xh, $_;

This guy worked like a charm, and git send-email, now, follows what the
man page says wrt the --in-reply-to usage.

I'll reformat the commit log, and repost the patch ASAP, if you are
OK with it.

-- Rafael


  reply	other threads:[~2020-06-25 18:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-24 19:55 [PATCH] send-email: restore --in-reply-to superseding behavior Rafael Aquini
2020-06-24 21:33 ` Junio C Hamano
2020-06-24 23:45   ` Rafael Aquini
2020-06-25 18:47     ` Rafael Aquini [this message]
2020-06-26  1:08       ` Carlo Arenas
2020-06-26 13:39         ` Rafael Aquini
2020-06-29 14:11   ` [PATCH v2] " Rafael Aquini
2020-07-01 22:10     ` Carlo Marcelo Arenas Belón

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200625184715.GC2117795@optiplex-lnx \
    --to=aquini@redhat.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.