git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: Aditya Garg <gargaditya08@live.com>,
	Junio C Hamano <gitster@pobox.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	Julian Swagemakers <julian@swagemakers.org>,
	Zi Yao <ziyao@disroot.org>, Jeff King <peff@peff.net>
Subject: Re: [PATCH] send-email: fix bug breaking shallow threading if the first patch is edited
Date: Fri, 23 May 2025 16:37:53 -0700	[thread overview]
Message-ID: <4dbb0dc7-52ef-4ffe-9215-de94fb448234@intel.com> (raw)
In-Reply-To: <73234CC5-8712-4B7B-94BE-F643345677BD@live.com>



On 5/23/2025 8:36 AM, Aditya Garg wrote:
> There is a bug in send-email that turns off shallow threading if
> some special conditions are there. Those conditions are:
> 
> 1. An --in-reply-to must be specified when sending the patch
> 2. When asked for confirmation before sending the first patch, the
>   user must edit the patch (pressing e and enter).
> 
> If these two conditions are fulfilled, the threading will turn off
> and all subsequent messages will become as replies to the
> Message-ID set in --in-reply-to, rather than becoming replies to
> the first patch.
> 
> The cause of this bug was very simple. There are many conditions
> that determine whether threading should be done or not. The
> relevant ones for this case are:
> 
> 1. --in-reply-to is not defined
> 2. $message_num is 1
> 
> If ANY ONE of these is fulfilled, threading will occur. Now, in
> our case, we have defined an --in-reply-to, so condition 1 is
> not fulfilled, and thus is omitted out. The only condition that
> can enable threading is $message_num being 1. As far as I
> understand, this condition was based on the assumption that the
> first message being send will have $message_num as 1, since in
> case of shallow threads, we just set in-reply-to only for the
> Message-ID of the first patch sent. But, in case we edit a patch,
> its $message_num increases by one, and thus, our second condition
> for threading is also not fulfilled, thus turning off threading.
> 

Why does editing a message change the message_num??? That feels like the
real bug to me..

> Luckily, the script also keeps count of the number of messages
> actually sent using the $num_sent variable. This was implemented
> for people who have set a particular batch size for emails. This
> is a more reliable indicator to track the actual first patch.
> 
> So, whenever the first patch is sent, $num_sent will become 1.
> If we replace the condition to use threading from $message_num
> to $num_sent=1, it will always be fulfilled irrespective of
> whether the user edits the first patch or not, and thus threading
> will turn on.
> 

Just from reading the commit message, this smells like a hack or
workaround to me.

I don't fully understand the logic in place as-is, and I think using
num_sent is a bit of a weird way to resolve this.

  parent reply	other threads:[~2025-05-23 23:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-23 15:36 [PATCH] send-email: fix bug breaking shallow threading if the first patch is edited Aditya Garg
2025-05-23 15:44 ` Aditya Garg
2025-05-23 22:19 ` Junio C Hamano
2025-05-23 23:37 ` Jacob Keller [this message]
2025-05-24 12:39 ` [PATCH v2] send-email: fix bug resulting in increased message number if a message " Aditya Garg
2025-05-25 17:12   ` [PATCH v3 0/2] send-email: fix threads breaking in case user edits emails and improvements to outlook ID fix Aditya Garg
2025-05-25 17:12     ` [PATCH v3 1/2] send-email: fix bug resulting in increased message number if a message is edited Aditya Garg
2025-05-25 18:49       ` Kristoffer Haugsbakk
2025-05-26 15:54         ` Aditya Garg
2025-05-25 17:12     ` [PATCH v3 2/2] send-email: show the new message id assigned by outlook in the logs Aditya Garg
  -- strict thread matches above, loose matches on Subject: below --
2025-05-24  2:56 [PATCH] send-email: fix bug breaking shallow threading if the first patch is edited Aditya Garg
2025-05-24  2:57 Aditya Garg

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=4dbb0dc7-52ef-4ffe-9215-de94fb448234@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=gargaditya08@live.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=julian@swagemakers.org \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    --cc=sunshine@sunshineco.com \
    --cc=ziyao@disroot.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).