On 29 May 2025, at 4:22 AM, Junio C Hamano [off-list ref] wrote:

Aditya Garg [off-list ref] writes:

quoted

Subject: Re: [PATCH v5 1/2] send-email: fix bug resulting in increased message number if a message is edited

Is this the same title Kristoffer said that it does not give much
meaningful information, to which you said you "have re-written the
whole message"?

I understood it as needing rewrite only of the body, and not the subject.

cf. [ref]

quoted

Whenever we send a thread of emails using send-email, a message number
is internally assigned to each email. This number is used to track the
order of the emails in the thread. Whenever a new message is processed
in a thread, the current script logic increases the message number by
one, which is intended.

But, if a message is edited and then resent, its message number again
gets increased. This is because the script uses the same logic to
process the edited message, which it uses to send the next message.

"increase" -> "increment" is more common to count up by one.

Ok

quoted

This minor bug is usually harmless, unless some special situations arise.
One such situation is when the first message in a thread is edited

Which makes it sound like you know of more than one bug but only
telling us about "one such situation" here.  If so, what are others?
If not, the phrasing in this paragraph is somewhat misleading.

I'll change it.

quoted

Here `$message_num` is the current message number, and `$in_reply_to` is
the Message-ID of the message to which the current message is a reply.
In case `--in-reply-to` is specified, the `$in_reply_to` variable
is set to the value of the `--in-reply-to` argument.

Whenever this whole set of conditions is true, the script sets the
`$in_reply_to` variable to the current message's ID. This is done to
ensure that the next message in the thread is a reply to this message.

OK.

quoted

To fix this bug, we need to ensure that the `$message_num` variable is
not increased by 1 when a message is edited and resent. We do this by
decreasing both the `$message_num` and `$message_id_serial` variables
by 1 whenever the request to edit a message is received. This way, the
next message in the thread will have the same message number as the
edited message. Therefore the threading will work as expected.

Hmph, isn't it more like "after editing Nth message, we re-process
that edited message, and we used to call that edited message N+1th,
which was wrong.  We now keep the same numbering and call the edited
message Nth (and the version before editing we didn't send, so there
is no risk of sending two Nth messages)"?  

Yes

quoted

The same logic has also been applied in case the user drops a single
message from the thread by choosing the "[n]o" option during
confirmation. By doing this, the next message in the thread is assigned
the message number of the dropped message, and thus the threading
works as expected.

OK.

The above explains why the patch needs to touch message_num.  It
would be evfen better if it described what the variable is used for,
exactly.

Whenever we send a thread of emails using send-email, a message number
is internally assigned to each email. This number is used to track the
order of the emails in the thread.

I had explained this in the starting.

   Side note: during the initial round of this change, I explained
   that $num_sent is the counter in the batch we are sending out
   (hence it is reset to 0 when a batch fills and the next batch
   starts).  If there is a similar concise and clear explanation of
   what $message_num?  "The number, counting from 1, of the message
   in the set of messages we are sending", or something, perhaps?

And none of the above justifies why this patch mucks with
message_id_serial.  Should it always be the same as message_num (in
which case the natural question is "why do we need both?")?

I had the similar question as well, but it also has special cases.

In case a person starts a thread using the --thread option of format-patch
and by mistake deletes the message id header of a patch, the serial number
starts with one from that patch.

Although honestly speaking, I myself am getting more confused as far as
serial number is concerned, and I think its best to drop it since its not a
breaking change unlike message number. It's just to ensure a unique message
ID to all patches, and having incremented numbers is ok.

Message number definitely needs a fix since it breaks threads.

If we can prove that message_num and message_id_serial must be
incremented in sync, it is OK to have a separate topic that unifies
these two variables into just a single message_num, but I'd prefer
not to see message_id_serial mentioned above and touched below at
all in this patch to fix the in_reply_to issue.

quoted

Signed-off-by: Aditya Garg <redacted>
---
git-send-email.perl | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 55b7e00d29..b09251c4fc 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl

@@ -1639,8 +1639,20 @@ sub send_message {
                default => $ask_default);
       die __("Send this email reply required") unless defined $_;
       if (/^n/i) {
+            # If we are skipping a message, we should make sure that
+            # the next message is treated as the successor to the
+            # previously sent message, and not the skipped message.
+            $message_num--;
+            $message_id_serial--;
           return 0;
       } elsif (/^e/i) {
+            # Since the same message will be sent again, we need to
+            # decrement the message number to the previous message.
+            # Otherwise, the edited message will be treated as a
+            # different message sent after the original non-edited
+            # message.
+            $message_num--;
+            $message_id_serial--;
           return -1;
       } elsif (/^q/i) {
           cleanup_compose_files();