# [PATCH v5 1/2] send-email: fix bug resulting in increased message number if a message is edited 5 messages from 2025-05-28 to 2025-05-29. Participants: Aditya Garg, Junio C Hamano. Thread: https://gitlist.dev/t/63537 ## Aditya Garg, 2025-05-28 06:39 Subject: [PATCH v5 1/2] send-email: fix bug resulting in increased message number if a message is edited Message-ID: <5103ea4034cd4a8438865a2d3da19a92dda54013.1748414082.git.gargaditya08@live.com> URL: https://gitlist.dev/e/5103ea4034cd4a8438865a2d3da19a92dda54013.1748414082.git.gargaditya08%40live.com In-Reply-To: ``` 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. This minor bug is usually harmless, unless some special situations arise. One such situation is when the first message in a thread is edited and resent, and an `--in-reply-to` argument is also passed to send-email. In this case, if the user has chosen shallow threading, the threading does not work as expected, and all messages become replies to the Message-ID specified in the `--in-reply-to` argument. The reason for this bug is hidden in the code for threading itself. if ($thread) { if ($message_was_sent && ($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 || $message_num == 1)) { $in_reply_to = $message_id; if (length $references > 0) { $references .= "\n $message_id"; } else { $references = "$message_id"; } } } 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. In case we specify an `--in-reply-to` argument, and have shallow threading, the only condition that can make this true is `$message_num == 1`, which is true for the first message in a thread. Thus, the `$in_reply_to` variable gets set to the first message's ID. For subsequent messages, the `$message_num` variable is always greater than 1, and the whole set of conditions is false. Therefore, the `$in_reply_to` variable remains as the first message's ID. This is what we expect in shallow threading. But if the user edits the first message and resends it, the `$message_num` variable gets increased by 1, and thus the condition `$message_num == 1` becomes false. This means that the `$in_reply_to` variable is not set to the first message's ID. As a result the next message in the thread is not a reply to the first message, but to the `--in-reply-to` argument, effectively breaking the threading. In case the user does not specify an `--in-reply-to` argument, the `!defined $in_reply_to` condition is true, and thus the `$in_reply_to` variable is set to the first message's ID, and the threading works as expected, regardless of the message number. Just like the `$message_num` variable, the `$message_id_serial` variable also increases by 1 whenever a new message is sent. This variable displays the message number in the Message-ID of the email. 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. 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. Signed-off-by: Aditya Garg --- 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(); -- 2.43.0 ``` ## Aditya Garg, 2025-05-28 06:39 Subject: [PATCH v5 0/2] send-email: fix threads breaking in case user edits emails and improvements to outlook ID fix. Message-ID: URL: https://gitlist.dev/e/cover.1748414082.git.gargaditya08%40live.com In-Reply-To: ``` Hi all, This patch series fixes two minor issues with git-send-email. The first patch fixes a bug that caused the message number to increase when a user edits an email. As a result of this bug, threads would break when a user edits an email. The second patch improves the logging of the new message ID assigned by Outlook when a user edits an email. v2 - Change the bugfix logic used to fix the threading bug. v3 - Add additional patch to improve the logging of the new message ID assigned by Outlook. v4 - Completely rewrite the commit message of the first patch to explain the bug in detail and how it is fixed. Also, add example logs to explain the second patch. v5 - Fix numerous spelling and grammatical errors in the commit message of the first patch. Aditya Garg (2): send-email: fix bug resulting in increased message number if a message is edited send-email: show the new message id assigned by outlook in the logs git-send-email.perl | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) Range-diff: 1: 63addf9d9b ! 1: 5103ea4034 send-email: fix bug resulting in increased message number if a message is edited @@ Commit message 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 is uses the send the next message. + process the edited message, which it uses to send the next message. - This minor bug is usually harmless unless some special situations arise. + This minor bug is usually harmless, unless some special situations arise. One such situation is when the first message in a thread is edited and resent, and an `--in-reply-to` argument is also passed to send-email. - In this case if the user has chosen shallow threading, the threading - does not work as expected, and all messaged become as replies to the + In this case, if the user has chosen shallow threading, the threading + does not work as expected, and all messages become replies to the Message-ID specified in the `--in-reply-to` argument. The reason for this bug is hidden in the code for threading itself. @@ Commit message } } - Here $message_num is the current message number, and $in_reply_to is + 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. @@ Commit message In case we specify an `--in-reply-to` argument, and have shallow threading, the only condition that can make this true is `$message_num == 1`, which is true for the first message in a thread. - Thus the $in_reply_to variable gets set to the first message's ID. + Thus, the `$in_reply_to` variable gets set to the first message's ID. For subsequent messages, the `$message_num` variable is always - greater than 1, and the whole set of conditions is false, and thus the - $in_reply_to variable remains as the first message's ID. This is what - we expect in shallow threading. But, in case the user edits the first - message and resends it, the `$message_num` variable gets increased by 1, - and thus the condition `$message_num == 1` becomes false. This means - that the `$in_reply_to` variable is not set to the first message's ID, - and thus the next message in the thread is not a reply to the first - message, but to the `--in-reply-to` argument, effectively breaking - the threading. + greater than 1, and the whole set of conditions is false. Therefore, the + `$in_reply_to` variable remains as the first message's ID. This is what + we expect in shallow threading. But if the user edits the first message + and resends it, the `$message_num` variable gets increased by 1, and + thus the condition `$message_num == 1` becomes false. This means that + the `$in_reply_to` variable is not set to the first message's ID. As a + result the next message in the thread is not a reply to the first + message, but to the `--in-reply-to` argument, effectively breaking the + threading. In case the user does not specify an `--in-reply-to` argument, the - !defined $in_reply_to condition is true, and thus the `$in_reply_to` + `!defined $in_reply_to` condition is true, and thus the `$in_reply_to` variable is set to the first message's ID, and the threading works - as expected, irrespective of what the message number is. + as expected, regardless of the message number. - Just like $message_num, $message_id_serial variable also increases by 1 - whenever a new message is sent. This variable displays the message - number is the Message-ID of the email. + Just like the `$message_num` variable, the `$message_id_serial` variable + also increases by 1 whenever a new message is sent. This variable + displays the message number in the Message-ID of the email. - So, in order 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 the $message_num and $message_id_serial - variable 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, and thus the threading will work as expected. + 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. 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 2: 72b3cd1bfb = 2: 6f2668de07 send-email: show the new message id assigned by outlook in the logs -- 2.43.0 ``` ## Aditya Garg, 2025-05-28 06:39 Subject: [PATCH v5 2/2] send-email: show the new message id assigned by outlook in the logs Message-ID: <6f2668de07ecd1558888f382ef5ebc8b8bb10f51.1748414082.git.gargaditya08@live.com> URL: https://gitlist.dev/e/6f2668de07ecd1558888f382ef5ebc8b8bb10f51.1748414082.git.gargaditya08%40live.com In-Reply-To: ``` Whenever an email is sent, send-email shows a log at last, which contains all the headers of the email that were received by the receipients. In case outlook changes the Message-ID, a log for the same is shown to the user, but that change is not reflected when the log containing all the headers is displayed. Here is an example of the log that is shown when outlook changes the Message-ID: Outlook reassigned Message-ID to: OK. Log says: Server: smtp.office365.com MAIL FROM: RCPT TO: From: Aditya Garg To: negahe7142@nomrista.com Subject: [PATCH] send-email: show the new message id assigned by outlook in the logs Date: Mon, 26 May 2025 20:28:36 +0530 Message-ID: <20250526145836.4825-1-gargaditya08@live.com> X-Mailer: git-send-email @GIT_VERSION@ MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Result: 250 This patch fixes this by modifying the $header variable, which is responsible for showing the logs at the end. Also, the log which states that the Message-ID has been changed will now be shown only when smtp-debug is enabled, since the main log having all of the headers is anyways displaying the new Message-ID. It should look like this after this patch: OK. Log says: Server: smtp.office365.com MAIL FROM: RCPT TO: From: Aditya Garg To: negahe7142@nomrista.com Subject: [PATCH] send-email: show the new message id assigned by outlook in the logs Date: Mon, 26 May 2025 20:29:22 +0530 Message-ID: X-Mailer: git-send-email @GIT_VERSION@ MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Result: 250 Signed-off-by: Aditya Garg --- git-send-email.perl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/git-send-email.perl b/git-send-email.perl index b09251c4fc..e8019c40ba 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1776,7 +1776,9 @@ sub send_message { if (is_outlook($smtp_server)) { if ($smtp->message =~ /<([^>]+)>/) { $message_id = "<$1>"; - printf __("Outlook reassigned Message-ID to: %s\n"), $message_id; + # Replace the original Message-ID in $header with the new one + $header =~ s/^(Message-ID:\s*).*\n/${1}$message_id\n/m; + printf __("Outlook reassigned Message-ID to: %s\n"), $message_id if $smtp->debug; } else { warn __("Warning: Could not retrieve Message-ID from server response.\n"); } -- 2.43.0 ``` ## Junio C Hamano, 2025-05-28 22:52 Subject: Re: [PATCH v5 1/2] send-email: fix bug resulting in increased message number if a message is edited Message-ID: URL: https://gitlist.dev/e/xmqqcybspcvf.fsf%40gitster.g In-Reply-To: <5103ea4034cd4a8438865a2d3da19a92dda54013.1748414082.git.gargaditya08@live.com> ``` Aditya Garg 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"? cf. > 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. > 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. > 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)"? > 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. 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?")? 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 > --- > 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(); ``` ## Aditya Garg, 2025-05-29 03:26 Subject: Re: [PATCH v5 1/2] send-email: fix bug resulting in increased message number if a message is edited Message-ID: URL: https://gitlist.dev/e/PN3PR01MB9597A24F38B01FE1F614D942B866A%40PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM In-Reply-To: ``` > On 29 May 2025, at 4:22 AM, Junio C Hamano wrote: > > Aditya Garg 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. > >> 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 >> --- >> 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(); ```