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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 is uses the 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 messaged become as 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, 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.

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, irrespective of what the message number is.

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.

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.

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 <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();
-- 
2.43.0






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux