Re: [PATCH] rebase -i: use same commit's message and date with f -C

> 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.

> 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(-)
> 
> 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
>   '
>