From: Phillip Wood Date: Tue, 23 Sep 2025 15:23:47 GMT Subject: Re: [PATCH] rebase -i: use same commit's message and date with f -C Message-ID: <1635b82d-a3aa-4b83-8d7b-e415945dbd2e@gmail.com> In-Reply-To: <92d4d585-09e9-4f1d-a471-1ad6b312fa61@app.fastmail.com> Hi Mathias On 23/09/2025 09:55, Mathias Rav wrote: > In `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. In that case I'd manually squash the later commit into the split commit as I don't think you cannot use "rebase --autosquash" to automatically squash a fixup commit into one of its descendants. I use "fixup -C" to amend and reword existing commits in the same way that I use an ordinary "fixup" to amend an existing commit. The only difference is that I'm rewording the commit message at the same time as I'm possibly changing the commit content. I use "fixup -C" to expanding the commit message, or fix typos and want to keep the original commit's authorship as I would do if I was not changing the commit message. I think the difference here is that the design of the "fixup" and "squash" commands assumes that they are fixing up an ancestor, not a descendant. Thanks Phillip > 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 > --- > > 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(-) > > 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 = use commit, but meld into previous commit\n" > "f, fixup [-C | -c] = 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 = run command (the rest of the line) using shell\n" > "b, break = stop here (continue rebase later with 'git rebase --continue')\n" > "d, drop = 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 > ' >