> On 29 May 2025, at 4:22 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Aditya Garg <gargaditya08@xxxxxxxx> writes:
>
>> 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. <PN3PR01MB95973C8CEC731B43816AB38CB865A@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
>
>> 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
>
>> 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.
>> 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.
>
>> 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
>
>> 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.
>
>> Signed-off-by: Aditya Garg <gargaditya08@xxxxxxxx>
>> ---
>> 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();
