[PATCH v2 2/2] rebase: remember fixup -c after skipping fixup/squash

Phillip Wood <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <3089979e2daf5bc8532008539e37695091dd10b2.1785080337.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <[email protected]>

When the final command in a chain of "fixup" and "squash" commands
is skipped, we should prompt the user to edit the commit message
if the chain contains a "fixup -c" command that was not skipped.
Unfortunately, commit_staged_changes() only looks for completed "squash"
commands and so does not prompt the user to edit the message. Fix
this by recording whether a fixup command has the "-c" flag set and
then checking whether we have seen either a "fixup -c" or a "squash"
command. Add regression tests for skipping a command in the middle
of the chain (which currently works but has no test coverage), and
for skipping the final command (which is fixed by this patch).

Signed-off-by: Phillip Wood <[email protected]>
---
 sequencer.c                     | 20 +++++++++++---
 t/t3437-rebase-fixup-options.sh | 47 +++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 4640ee9b7f5..1a0a283b42c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1924,6 +1924,13 @@ static int seen_squash(struct replay_ctx *ctx)
 {
 	return starts_with(ctx->current_fixups.buf, "squash") ||
 		strstr(ctx->current_fixups.buf, "\nsquash");
+}
+
+/* Does the current fixup chain contain a "fixup -c" command? */
+static int seen_fixup_edit_msg(struct replay_ctx *ctx)
+{
+	return starts_with(ctx->current_fixups.buf, "fixup -c") ||
+		strstr(ctx->current_fixups.buf, "\nfixup -c");
 }
 
 static void update_comment_bufs(struct strbuf *buf1, struct strbuf *buf2, int n)
@@ -2148,9 +2155,14 @@ static int update_squash_messages(struct repository *r,
 	strbuf_release(&buf);
 
 	if (!res) {
-		strbuf_addf(&ctx->current_fixups, "%s%s %s",
+		const char *fixup_flag = "";
+
+		if (is_fixup_flag(command, flag) && (flag & TODO_EDIT_FIXUP_MSG))
+			fixup_flag = " -c";
+
+		strbuf_addf(&ctx->current_fixups, "%s%s%s %s",
 			    ctx->current_fixups.len ? "\n" : "",
-			    command_to_string(command),
+			    command_to_string(command), fixup_flag,
 			    oid_to_hex(&commit->object.oid));
 		res = write_message(ctx->current_fixups.buf,
 				    ctx->current_fixups.len,
@@ -5391,8 +5403,8 @@ static int commit_staged_changes(struct repository *r,
 				 * message, no need to bother the user with
 				 * opening the commit message in the editor.
 				 */
-				if (!starts_with(p, "squash ") &&
-				    !strstr(p, "\nsquash "))
+				if (!seen_squash(ctx) &&
+				    !seen_fixup_edit_msg(ctx))
 					flags = (flags & ~EDIT_MSG) | CLEANUP_MSG;
 			} else if (is_fixup(peek_command(todo_list, 0))) {
 				/*
diff --git a/t/t3437-rebase-fixup-options.sh b/t/t3437-rebase-fixup-options.sh
index 5d306a47692..a4b2a631654 100755
--- a/t/t3437-rebase-fixup-options.sh
+++ b/t/t3437-rebase-fixup-options.sh
@@ -184,6 +184,53 @@ test_expect_success 'multiple fixup -c opens editor once' '
 	get_author HEAD >actual-author &&
 	test_cmp expected-author actual-author &&
 	test_commit_message HEAD expected-message
+'
+
+test_expect_success 'fixup -c is remembered after skipping final fixup' '
+	test_when_finished "test_might_fail git rebase --abort" &&
+	cat >todo <<-\EOF &&
+	pick B
+	fixup -c A1
+	fixup A3
+	EOF
+	(
+		set_fake_editor &&
+		set_replace_editor todo &&
+		test_must_fail git rebase -i A A &&
+		git show && cat .git/rebase-merge/message-squash &&
+		FAKE_COMMIT_AMEND=edited git rebase --skip
+	) &&
+	test_commit_message HEAD <<-\EOF
+	new subject
+
+	new
+	body
+
+	edited
+	EOF
+'
+test_expect_success 'fixup -c is remembered after skipping later fixup' '
+	test_when_finished "test_might_fail git rebase --abort" &&
+	cat >todo <<-\EOF &&
+	pick B
+	fixup -c A1
+	fixup A3
+	fixup A2
+	EOF
+	(
+		set_fake_editor &&
+		set_replace_editor todo &&
+		test_must_fail git rebase -i A A &&
+		FAKE_COMMIT_AMEND=edited git rebase --skip
+	) &&
+	test_commit_message HEAD <<-\EOF
+	new subject
+
+	new
+	body
+
+	edited
+	EOF
 '
 
 test_expect_success 'sequence squash, fixup & fixup -c gives combined message' '
-- 
2.54.0.200.gfd8d68259e3
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.