rebase --skip: clean up commit message after a failed fixup/squash

During a series of fixup/squash commands, the interactive rebase builds
up a commit message with comments. This will be presented to the user in
the editor if at least one of those commands was a `squash`.

In any case, the commit message will be cleaned up eventually, removing
all those intermediate comments, in the final step of such a
fixup/squash chain.

However, if the last fixup/squash command in such a chain fails with
merge conflicts, and if the user then decides to skip it (or resolve it
to a clean worktree and then continue the rebase), the current code
fails to clean up the commit message.

This commit fixes that behavior.

The fix is quite a bit more involved than meets the eye because it is
not only about the question whether we are `git rebase --skip`ing a
fixup or squash. It is also about removing the skipped fixup/squash's
commit message from the accumulated commit message. And it is also about
the question whether we should let the user edit the final commit
message or not ("Was there a squash in the chain *that was not
skipped*?").

For example, in this case we will want to fix the commit message, but
not open it in an editor:

	pick	<- succeeds
	fixup	<- succeeds
	squash	<- fails, will be skipped

This is where the newly-introduced `current-fixups` file comes in real
handy. A quick look and we can determine whether there was a non-skipped
squash. We only need to make sure to keep it up to date with respect to
skipped fixup/squash commands. As a bonus, we can even avoid committing
unnecessarily, e.g. when there was only one fixup, and it failed, and
was skipped.

To fix only the bug where the final commit message was not cleaned up
properly, but without fixing the rest, would have been more complicated
than fixing it all in one go, hence this commit lumps together more than
a single concern.

For the same reason, this commit also adds a bit more to the existing
test case for the regression we just fixed.

The diff is best viewed with --color-moved.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 31 insertions, 4 deletions

diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 69cb079dd1..e9fd029160 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -88,14 +88,14 @@ test_expect_success 'rebase passes merge strategy options correctly' '
 	git rebase --continue
 '
 
-test_expect_failure '--skip after failed fixup cleans commit message' '
+test_expect_success '--skip after failed fixup cleans commit message' '
 	test_when_finished "test_might_fail git rebase --abort" &&
 	git checkout -b with-conflicting-fixup &&
 	test_commit wants-fixup &&
 	test_commit "fixup! wants-fixup" wants-fixup.t 1 wants-fixup-1 &&
 	test_commit "fixup! wants-fixup" wants-fixup.t 2 wants-fixup-2 &&
 	test_commit "fixup! wants-fixup" wants-fixup.t 3 wants-fixup-3 &&
-	test_must_fail env FAKE_LINES="1 fixup 2 fixup 4" \
+	test_must_fail env FAKE_LINES="1 fixup 2 squash 4" \
 		git rebase -i HEAD~4 &&
 
 	: now there is a conflict, and comments in the commit message &&
@@ -103,11 +103,38 @@ test_expect_failure '--skip after failed fixup cleans commit message' '
 	grep "fixup! wants-fixup" out &&
 
 	: skip and continue &&
-	git rebase --skip &&
+	echo "cp \"\$1\" .git/copy.txt" | write_script copy-editor.sh &&
+	(test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) &&
+
+	: the user should not have had to edit the commit message &&
+	test_path_is_missing .git/copy.txt &&
 
 	: now the comments in the commit message should have been cleaned up &&
 	git show HEAD >out &&
-	! grep "fixup! wants-fixup" out
+	! grep "fixup! wants-fixup" out &&
+
+	: now, let us ensure that "squash" is handled correctly &&
+	git reset --hard wants-fixup-3 &&
+	test_must_fail env FAKE_LINES="1 squash 4 squash 2 squash 4" \
+		git rebase -i HEAD~4 &&
+
+	: the first squash failed, but there are two more in the chain &&
+	(test_set_editor "$PWD/copy-editor.sh" &&
+	 test_must_fail git rebase --skip) &&
+
+	: not the final squash, no need to edit the commit message &&
+	test_path_is_missing .git/copy.txt &&
+
+	: The first squash was skipped, therefore: &&
+	git show HEAD >out &&
+	test_i18ngrep "# This is a combination of 2 commits" out &&
+
+	(test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) &&
+	git show HEAD >out &&
+	test_i18ngrep ! "# This is a combination" out &&
+
+	: Final squash failed, but there was still a squash &&
+	test_i18ngrep "# This is a combination of 2 commits" .git/copy.txt
 '
 
 test_expect_success 'setup rerere database' '