Re: [PATCH v3 1/2] commit: allow -m/-F for all kinds of --fixup

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Erik

On 22/09/2026 14:04, erik@cervined.in wrote:

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 205fbd57e3..ffb8644653 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -816,18 +816,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>   	if (have_option_m && !fixup_message) {
>   		strbuf_addbuf(&sb, &message);
>   		hook_arg1 = "message";
> -	} else if (logfile && !strcmp(logfile, "-")) {
> +	} else if (logfile && !fixup_message && !strcmp(logfile, "-")) {
>   		if (isatty(0))
>   			fprintf(stderr, _("(reading log message from standard input)\n"));
>   		if (strbuf_read(&sb, 0, 0) < 0)
>   			die_errno(_("could not read log from standard input"));
>   		hook_arg1 = "message";
It is a bit unfortunate that we have to guard these with 
"!fixup_message" and duplicate the inside the "else if (fixup_message)" 
lower down, but I don't see an easy way of avoiding it. I guess we could 
factor it out into a helper function but I'm not sure if it is worth the 
effort.

> @@ -849,20 +849,21 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>   		hook_arg1 = "message";
>   
>   		/*
> -		 * Only `-m` commit message option is checked here, as
> -		 * it supports `--fixup` to append the commit message.
> -		 *
> -		 * The other commit message options `-c`/`-C`/`-F` are
> -		 * incompatible with all the forms of `--fixup` and
> -		 * have already errored out while parsing the `git commit`
> -		 * options.
> +		 * Only `-m` and `-F` are handled here. `-c`/`-C` are
> +		 * incompatible with --fixup and have already errored out
> +		 * during option parsing.
>   		 */
> -		if (have_option_m && !strcmp(fixup_prefix, "fixup"))
> +		if (have_option_m) {
>   			strbuf_addbuf(&sb, &message);
> -
> -		if (!strcmp(fixup_prefix, "amend")) {
> -			if (have_option_m)
> -				die(_("options '%s' and '%s:%s' cannot be used together"), "-m", "--fixup", fixup_message);
> +		} else if (logfile && !strcmp(logfile, "-")) {
> +			if (isatty(0))
> +				fprintf(stderr, _("(reading log message from standard input)\n"));
> +			if (strbuf_read(&sb, 0, 0) < 0)
> +				die_errno(_("could not read log from standard input"));
> +		} else if (logfile) {
> +			if (strbuf_read_file(&sb, logfile, 0) < 0)
> +				die_errno(_("could not read log file '%s'"), logfile);

This all looks good - we add the appropriate "fixup!" style subject line 
and then add the message from the commandline or file.

The test changes below look good too.

Thanks

Phillip


> +		} else if (!strcmp(fixup_prefix, "amend")) {
>   			prepare_amend_commit(commit, &sb, &ctx);
>   		}
>   	} else if (!stat(git_path_merge_msg(the_repository), &statbuf)) {
> @@ -1365,9 +1366,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   	}
>   	if (fixup_message && squash_message)
>   		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
> -	die_for_incompatible_opt4(!!use_message, "-C",
> +	die_for_incompatible_opt3(!!use_message, "-C",
>   				  !!edit_message, "-c",
> -				  !!logfile, "-F",
>   				  !!fixup_message, "--fixup");
>   	die_for_incompatible_opt4(have_option_m, "-m",
>   				  !!edit_message, "-c",
> diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
> index 5e782ad8d7..9b2ecb1356 100755
> --- a/t/t7500-commit-template-squash-signoff.sh
> +++ b/t/t7500-commit-template-squash-signoff.sh
> @@ -384,18 +384,24 @@ test_expect_success '--fixup=reword: ignores staged changes' '
>   	test_cmp foo actual
>   '
>   
> -test_expect_success '--fixup=reword: error out with -m option' '
> +test_expect_success 'commit --fixup=reword: works with -m' '
>   	commit_for_rebase_autosquash_setup &&
> -	echo "fatal: options '\''-m'\'' and '\''--fixup:reword'\'' cannot be used together" >expect &&
> -	test_must_fail git commit --fixup=reword:HEAD~ -m "reword commit message" 2>actual &&
> -	test_cmp expect actual
> +	git commit --fixup=reword:HEAD~ -m "reword commit message" &&
> +	test_commit_message HEAD <<-EOF
> +	amend! $(git log -1 --format=%s HEAD~2)
> +
> +	reword commit message
> +	EOF
>   '
>   
> -test_expect_success '--fixup=amend: error out with -m option' '
> +test_expect_success 'commit --fixup=amend: works with -m' '
>   	commit_for_rebase_autosquash_setup &&
> -	echo "fatal: options '\''-m'\'' and '\''--fixup:amend'\'' cannot be used together" >expect &&
> -	test_must_fail git commit --fixup=amend:HEAD~ -m "amend commit message" 2>actual &&
> -	test_cmp expect actual
> +	git commit --fixup=amend:HEAD~ -m "amend commit message" &&
> +	test_commit_message HEAD <<-EOF
> +	amend! $(git log -1 --format=%s HEAD~2)
> +
> +	amend commit message
> +	EOF
>   '
>   
>   test_expect_success 'consecutive amend! commits remove amend! line from commit msg body' '
> @@ -432,6 +438,13 @@ test_expect_success 'deny to create amend! commit if its commit msg body is empt
>   	test_cmp expected actual
>   '
>   
> +test_expect_success 'deny to create amend! commit if -m is empty' '
> +	commit_for_rebase_autosquash_setup &&
> +	echo "Aborting commit due to empty commit message body." >expect &&
> +	test_must_fail git commit --fixup=amend:HEAD~ -m "" 2>actual &&
> +	test_cmp expect actual
> +'
> +
>   test_expect_success 'amend! commit allows empty commit msg body with --allow-empty-message' '
>   	commit_for_rebase_autosquash_setup &&
>   	cat >expected <<-EOF &&
> @@ -468,10 +481,26 @@ test_expect_success '--fixup=reword: give error with pathsec' '
>   	test_cmp expect actual
>   '
>   
> -test_expect_success '--fixup=reword: -F give error message' '
> -	echo "fatal: options '\''-F'\'' and '\''--fixup'\'' cannot be used together" >expect &&
> -	test_must_fail git commit --fixup=reword:HEAD~ -F msg  2>actual &&
> -	test_cmp expect actual
> +test_expect_success 'commit --fixup works with -F' '
> +	commit_for_rebase_autosquash_setup &&
> +	echo "message" >msgfile &&
> +	git commit --fixup HEAD~ -F msgfile &&
> +	test_commit_message HEAD <<-EOF
> +	fixup! $(git log -1 --format=%s HEAD~2)
> +
> +	message
> +	EOF
> +'
> +
> +test_expect_success 'commit --fixup=reword: works with -F' '
> +	commit_for_rebase_autosquash_setup &&
> +	echo "message from file" >msgfile &&
> +	git commit --fixup=reword:HEAD~ -F msgfile &&
> +	test_commit_message HEAD <<-EOF
> +	amend! $(git log -1 --format=%s HEAD~2)
> +
> +	$(cat msgfile)
> +	EOF
>   '
>   
>   test_expect_success 'commit --squash works with -F' '
> @@ -526,8 +555,7 @@ test_expect_success 'invalid message options when using --fixup' '
>   	git add foo &&
>   	test_must_fail git commit --fixup HEAD~1 --squash HEAD~2 &&
>   	test_must_fail git commit --fixup HEAD~1 -C HEAD~2 &&
> -	test_must_fail git commit --fixup HEAD~1 -c HEAD~2 &&
> -	test_must_fail git commit --fixup HEAD~1 -F log
> +	test_must_fail git commit --fixup HEAD~1 -c HEAD~2
>   '
>   
>   cat >expected-template <<EOF