Mathias RavIn `git rebase -i` with the fixup command, the -C flag controls whether the commit message is taken from the previous or current commit, but currently the author name, email and date are always taken from the previous commit. The fixup command is used to squash two commits where one commit has a good message and the other's message does not matter, and it is usually also the case that the commit with the good message is the one that has the good authorship information; the other is a fixup commit that was presumably made by the user moments ago, whereas the commit with the good message is the one whose date should be kept.
Most of the time, a fixup commit is made on top of the commit to be fixed up, in which case the rebase -i fixup command is used without -C. The fixup -C case arises when an earlier commit in the branch is split, leaving part of the commit to be squashed into a later commit, in which case fixup -C would be expected to keep the date on the later commit, and discard the author date of the ephemeral newly split commit.
Change the behavior so that fixup with -C takes both message and author from the current commit, instead of taking the author from the previous.
Tweak try_to_commit to allow specifying author in addition to AMEND_MSG, and pass author from the current commit in do_pick_commit in `f -C`.
Tweak the help text in `git rebase -i` to reflect the changed behavior.
Add a test that ensures that the author metadata for the second current commit is kept, and remove some author metadata checks from other tests that now fail since the author metadata is different (as intended).
Signed-off-by: Mathias Rav <m@git.strova.dk> ---
I described my own workflow for fixup -C above, and it's the only use of fixup -C I'm aware of.
If the current behavior of keeping message from one and author from another is useful in someone else's workflow, then I'm happy to be enlightened.
Correct author dates are certainly more nice-to-have than need-to-have in most git workflows, but I think it's worthwhile to have git go the extra mile here.
rebase-interactive.c | 4 ++-- sequencer.c | 5 +++-- t/t3437-rebase-fixup-options.sh | 15 ++++++++++----- 3 files changed, 15 insertions(+), 9 deletions(-)
Show changes to 3 files +15 −9
rebase-interactive.c, sequencer.c, t/t3437-rebase-fixup-options.sh
diff --git a/rebase-interactive.c b/rebase-interactive.c index 809f76a87b..dd303168c2 100644 --- a/rebase-interactive.c +++ b/rebase-interactive.c @@ -53,8 +53,8 @@ void append_todo_help(int command_count, "s, squash <commit> = use commit, but meld into previous commit\n" "f, fixup [-C | -c] <commit> = like \"squash\" but keep only the previous\n" " commit's log message, unless -C is used, in which case\n" -" keep only this commit's message; -c is same as -C but\n" -" opens the editor\n" +" keep this commit's message and date; -c is same as -C\n" +" but opens the editor\n" "x, exec <command> = run command (the rest of the line) using shell\n" "b, break = stop here (continue rebase later with 'git rebase --continue')\n" "d, drop <commit> = remove commit\n" diff --git a/sequencer.c b/sequencer.c index aaf2e4df64..80209b6b07 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1560,7 +1560,8 @@ static int try_to_commit(struct repository *r, strbuf_addstr(msg, orig_message); hook_commit = "HEAD"; } - author = amend_author = get_author(message); + if (!author) + author = amend_author = get_author(message); repo_unuse_commit_buffer(r, current_head, message); if (!author) { @@ -2419,7 +2420,7 @@ static int do_pick_commit(struct repository *r, strbuf_addstr(&ctx->message, oid_to_hex(&commit->object.oid)); strbuf_addstr(&ctx->message, ")\n"); } - if (!is_fixup(command)) + if (is_fixup_flag(command, item->flags) || !is_fixup(command)) author = get_author(msg.message); } ctx->have_message = 1; diff --git a/t/t3437-rebase-fixup-options.sh b/t/t3437-rebase-fixup-options.sh index 5d306a4769..2361d3fb78 100755 --- a/t/t3437-rebase-fixup-options.sh +++ b/t/t3437-rebase-fixup-options.sh @@ -85,6 +85,15 @@ test_expect_success 'simple fixup -C works' ' test_commit_message HEAD -m "A2" ' +test_expect_success 'fixup -C keeps second commit date' ' + test_when_finished "test_might_fail git rebase --abort" && + git checkout --detach A2 && + get_author HEAD >expect && + FAKE_LINES="1 fixup_-C 2" git rebase -i B && + get_author HEAD >actual && + test_cmp expect actual +' + test_expect_success 'simple fixup -c works' ' test_when_finished "test_might_fail git rebase --abort" && git checkout --detach A2 && @@ -105,9 +114,7 @@ test_expect_success 'fixup -C removes amend! from message' ' FAKE_LINES="1 fixup_-C 2" git rebase -i A && test_cmp_rev HEAD^ A && test_cmp_rev HEAD^{tree} A1^{tree} && - test_commit_message HEAD expected-message && - get_author HEAD >actual-author && - test_cmp expected-author actual-author + test_commit_message HEAD expected-message ' test_expect_success 'fixup -C with conflicts gives correct message' ' @@ -181,8 +188,6 @@ test_expect_success 'multiple fixup -c opens editor once' ' EXPECT_HEADER_COUNT=4 \ git rebase -i A && test_cmp_rev HEAD^ A && - get_author HEAD >actual-author && - test_cmp expected-author actual-author && test_commit_message HEAD expected-message '
-- 2.51.0
