[PATCH 0/1] commit: allow -m/-F with --fixup=amend: or reword:

[PATCH 0/1] commit: allow -m/-F with --fixup=amend: or reword:

[erik]

From: Erik Cervin-Edin <erik@cervined.in>

The commit --fixup=reword: (and --fixup:amend) options are powerful but
currently not well-suited for non-interactive workflows.

I often find myself hacking away on a branch and the last thing I do is
finalize and formulate the commit messages. One of the current ways of
doing this is running an interactive rebase and picking the commits in
your branch to reword. However, doing this requires you to linearly go
through the messages and edit them one by one. The other options which
allows more flexible editing is to generate linear patches -- but this
trades editing freedom for branch topology freedom and has its own
drawbacks.

The --fixup=reword: flag introduced in 494d314a05 (commit: add
amend suboption to --fixup to create amend! commit, 2021-03-15),
adds a third workflow which allows rewording commits without initiating
a rebase and from the comfort of the HEAD of the branch. However, doing
such editing is only possible using $EDITOR, which restricts its use in
some workflows.

When amend:/reword: were introduced in Charvi's series, -m support
for amend fixups was discussed but not pursued
(xmqqwnuvsw0d.fsf@gitster.g and xmqqczwmsjzl.fsf@gitster.g):

On Fri, 26 Feb 2021 11:32:30 -0800, Junio C Hamano wrote:
> >> > +                     if (have_option_m)
> >> > +                             die(_("cannot combine -m with --fixup:%s"), fixup_message);
> >> > +                     else
> >> > +                             prepare_amend_commit(commit, &sb, &ctx);
> >>
> >> Hmph, why is -m so special?  Should we allow --fixup=amend:<cmd>
> >> with -F (or -c/-C for that matter), or are these other options
> >> caught at a lot higher layer already and we do not have to check
> >> them here?
> >
> > yes, those options are caught earlier and give the error as below:
> > "Only one of -c/-C/-F/--fixup can be used."
> > and only `-m` is checked over here.
>
> And the reason why -m cannot be checked early is because we do not
> recognize which kind of "fixup" we are doing when "only one of
> -c/-C/-F/--fixup" check is made before this function is called?
>
> OK.  I wonder if we can tell which kind of fixup we are doing much
> earlier, though.  Then we could extend it to say "Only one of
> -c/-C/-F/-m/--fixup=amend:<commit> can be used", etc., and we do not
> have to have this "only -m is checked here, everything else is
> checked earlier" curiosity.  But I do not know if such a change is
> necessarily an improvement.  I guess a better "fix" would probably
> be to add a comment to this function where it only checks for "-m"
> and tell readers why -c/-C/-F do not have to be checked here.

This patch picks up that thread by allowing both -m and -F for
amend/reword fixups, bypassing the need for an interactive editor.
This makes it practical to, for example, write replacement messages in
files and batch-apply them as reword fixups without stepping through
each one interactively. It's also friendly to AI agents who have a hard time
editing text using a non-interactive $EDITOR.

Allowing -c/-C was also considered but left out of this patch -- it can
be added in a re-roll if reviewers think it's worthwhile. I could see it
being useful, for example if you want to use git notes as a re-write
commit message channel. Since this is my first patch I intentionally
thought it best to start small.

Erik Cervin-Edin (1):
  commit: allow -m/-F with --fixup=amend: or reword:

 Documentation/git-commit.adoc             | 13 +++--
 builtin/commit.c                          | 41 ++++++++++----
 t/t7500-commit-template-squash-signoff.sh | 67 +++++++++++++++++++----
 3 files changed, 92 insertions(+), 29 deletions(-)

-- 
2.54.0.772.g683d7313b1

[PATCH 1/1] commit: allow -m/-F with --fixup=amend: or reword:

[erik]

From: Erik Cervin-Edin <erik@cervined.in>

--fixup=amend: and --fixup=reword: require an editor to supply the
replacement commit message. The -m and -F flags are rejected: -m is
caught by a die() in prepare_to_commit(), and -F is caught by
die_for_incompatible_opt4() which groups -F with --fixup as mutually
exclusive. This makes these modes unusable in non-interactive
workflows -- notably AI coding agents.

When the amend suboption was introduced in 494d314a05 (commit: add
amend suboption to --fixup to create amend! commit, 2021-03-15),
-m support for amend fixups was discussed but not pursued, and -F
was already caught by the higher-layer incompatibility check grouping
it with --fixup.

Allow -m and -F to supply the replacement message body for amend and
reword fixups. When provided, bypass the editor and directly use the
user's message as the body, replacing the original commit's message. For
-F, the file contents are read into the message strbuf and then handled
identically to -m.

Plain --fixup (without amend: or reword:) continues to reject -F but
still accepts -m (even though it's practically a no-op).

Signed-off-by: Erik Cervin Edin <erik@cervined.in>
---
 Documentation/git-commit.adoc             | 13 +++--
 builtin/commit.c                          | 41 ++++++++++----
 t/t7500-commit-template-squash-signoff.sh | 67 +++++++++++++++++++----
 3 files changed, 92 insertions(+), 29 deletions(-)

diff --git a/Documentation/git-commit.adoc b/Documentation/git-commit.adoc
index 8329c1034b..9478d5d265 100644
--- a/Documentation/git-commit.adoc
+++ b/Documentation/git-commit.adoc
@@ -111,12 +111,13 @@ commit, but the additional commentary will be thrown away once the
 The commit created by `--fixup=amend:<commit>` is similar but its
 title is instead prefixed with "amend!". The log message of
 _<commit>_ is copied into the log message of the "amend!" commit and
-opened in an editor so it can be refined. When `git rebase
---autosquash` squashes the "amend!" commit into _<commit>_, the
-log message of _<commit>_ is replaced by the refined log message
-from the "amend!" commit. It is an error for the "amend!" commit's
-log message to be empty unless `--allow-empty-message` is
-specified.
+opened in an editor so it can be refined. The replacement message may
+also be supplied directly using `-m` or `-F`, bypassing the need to open
+an editor. When `git rebase --autosquash` squashes the "amend!" commit
+into _<commit>_, the log message of _<commit>_ is replaced by the
+refined log message from the "amend!" commit. It is an error for the
+"amend!" commit's log message to be empty unless `--allow-empty-message`
+is specified.
 +
 `--fixup=reword:<commit>` is shorthand for `--fixup=amend:<commit>
  --only`. It creates an "amend!" commit with only a log message
diff --git a/builtin/commit.c b/builtin/commit.c
index 28f6174503..269c2d782b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -837,21 +837,19 @@ 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.
+		 * `-m` (and `-F`, converted to `-m` earlier for
+		 * amend/reword) appends the message body here.
+		 * `-c`/`-C` are still incompatible with all forms
+		 * of `--fixup`.
 		 */
 		if (have_option_m && !strcmp(fixup_prefix, "fixup"))
 			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);
-			prepare_amend_commit(commit, &sb, &ctx);
+				strbuf_addbuf(&sb, &message);
+			else
+				prepare_amend_commit(commit, &sb, &ctx);
 		}
 	} else if (!stat(git_path_merge_msg(the_repository), &statbuf)) {
 		size_t merge_msg_start;
@@ -1338,10 +1336,12 @@ 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_opt3(!!use_message, "-C",
+				  !!edit_message, "-c",
+				  !!logfile, "-F");
 	die_for_incompatible_opt4(have_option_m, "-m",
 				  !!edit_message, "-c",
 				  !!use_message, "-C",
@@ -1410,6 +1410,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		}
 	}
 
+	if (logfile && fixup_message && !strcmp(fixup_prefix, "fixup"))
+		die(_("options '%s' and '%s' cannot be used together"), "-F", "--fixup");
+
 	if (0 <= edit_flag)
 		use_editor = edit_flag;
 
@@ -1821,6 +1824,22 @@ int cmd_commit(int argc,
 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
 					  builtin_commit_usage,
 					  prefix, current_head, &s);
+
+	if (logfile && fixup_message && !strcmp(fixup_prefix, "amend")) {
+		if (!strcmp(logfile, "-")) {
+			if (isatty(0))
+				fprintf(stderr, _("(reading log message from standard input)\n"));
+			if (strbuf_read(&message, 0, 0) < 0)
+				die_errno(_("could not read log from standard input"));
+		} else {
+			if (strbuf_read_file(&message, logfile, 0) < 0)
+				die_errno(_("could not read log file '%s'"), logfile);
+		}
+		strbuf_complete_line(&message);
+		have_option_m = 1;
+		FREE_AND_NULL(logfile);
+	}
+
 	if (trailer_args.nr)
 		trailer_config_init();
 
diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
index 66aff8e097..b7579ad789 100755
--- a/t/t7500-commit-template-squash-signoff.sh
+++ b/t/t7500-commit-template-squash-signoff.sh
@@ -384,18 +384,28 @@ 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 '--fixup=amend: with -m option' '
 	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
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	amend commit message
+	EOF
+	git commit --fixup=amend:HEAD~ -m "amend commit message" &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
 '
 
-test_expect_success '--fixup=amend: error out with -m option' '
+test_expect_success '--fixup=reword: with -m option' '
 	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
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	reword commit message
+	EOF
+	git commit --fixup=reword:HEAD~ -m "reword commit message" &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
 '
 
 test_expect_success 'consecutive amend! commits remove amend! line from commit msg body' '
@@ -432,6 +442,12 @@ test_expect_success 'deny to create amend! commit if its commit msg body is empt
 	test_cmp expected actual
 '
 
+test_expect_success '--fixup=amend: -m with empty message aborts' '
+	commit_for_rebase_autosquash_setup &&
+	test_must_fail git commit --fixup=amend:HEAD~ -m "" 2>err &&
+	test_grep "empty commit message body" err
+'
+
 test_expect_success 'amend! commit allows empty commit msg body with --allow-empty-message' '
 	commit_for_rebase_autosquash_setup &&
 	cat >expected <<-EOF &&
@@ -468,10 +484,37 @@ 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 '--fixup=reword: with -F option' '
+	commit_for_rebase_autosquash_setup &&
+	echo "message from file" >msgfile &&
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	message from file
+	EOF
+	git commit --fixup=reword:HEAD~ -F msgfile &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '--fixup=amend: with -F option' '
+	commit_for_rebase_autosquash_setup &&
+	echo "amend message from file" >msgfile &&
+	cat >expected <<-EOF &&
+	amend! $(git log -1 --format=%s HEAD~)
+
+	amend message from file
+	EOF
+	git commit --fixup=amend:HEAD~ -F msgfile &&
+	get_commit_msg HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '-F with plain --fixup still errors' '
+	commit_for_rebase_autosquash_setup &&
+	echo "message" >msgfile &&
+	test_must_fail git commit --fixup HEAD~ -F msgfile 2>err &&
+	test_grep "cannot be used together" err
 '
 
 test_expect_success 'commit --squash works with -F' '
-- 
2.54.0.772.g683d7313b1

Re: [PATCH 1/1] commit: allow -m/-F with --fixup=amend: or reword:

[Junio C Hamano]

erik@cervined.in writes:

> From: Erik Cervin-Edin <erik@cervined.in>

The name on this overriding in-body From: line, and the name on
Signed-off-by: line below, must match.  Please pick a name with or
without hyphen and stick to it.

> --fixup=amend: and --fixup=reword: require an editor to supply the
> replacement commit message. The -m and -F flags are rejected: -m is
> caught by a die() in prepare_to_commit(), and -F is caught by
> die_for_incompatible_opt4() which groups -F with --fixup as mutually
> exclusive. This makes these modes unusable in non-interactive
> workflows -- notably AI coding agents.

"Unusable" may be stronger than reality, as you can make creatie use
of GIT_EDITOR to achieve what you want.  "awkward" or "poorly suited"
would be more fitting.

> Plain --fixup (without amend: or reword:) continues to reject -F but
> still accepts -m (even though it's practically a no-op).

Is it "practically a no-op"?  Wouldn't

   $ git commit --fixup <commit> -m "message body"

be useful to leave a message in the resulting commit, which is later
to be squashed into the named <commit>?  Actually squashing with "fixup!"
may lose the message supplied here, but wouldn't people use this
facility to more easily identify what each of the fixups are about?

For the same reason, "-F" would be just as useful as "-m" in this context,
and it feels a bit inconsistent to allow one while rejecting the other.

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 28f6174503..269c2d782b 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -837,21 +837,19 @@ 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.
> +		 * `-m` (and `-F`, converted to `-m` earlier for
> +		 * amend/reword) appends the message body here.
> +		 * `-c`/`-C` are still incompatible with all forms
> +		 * of `--fixup`.
>  		 */
>  		if (have_option_m && !strcmp(fixup_prefix, "fixup"))
>  			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);

Good that you got rid of this overly long die() message line.

> -			prepare_amend_commit(commit, &sb, &ctx);
> +				strbuf_addbuf(&sb, &message);
> +			else
> +				prepare_amend_commit(commit, &sb, &ctx);
>  		}
>  	} else if (!stat(git_path_merge_msg(the_repository), &statbuf)) {
>  		size_t merge_msg_start;
> @@ -1338,10 +1336,12 @@ 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_opt3(!!use_message, "-C",
> +				  !!edit_message, "-c",
> +				  !!logfile, "-F");
>  	die_for_incompatible_opt4(have_option_m, "-m",
>  				  !!edit_message, "-c",
>  				  !!use_message, "-C",
> @@ -1410,6 +1410,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  		}
>  	}
>  
> +	if (logfile && fixup_message && !strcmp(fixup_prefix, "fixup"))
> +		die(_("options '%s' and '%s' cannot be used together"), "-F", "--fixup");
> +
>  	if (0 <= edit_flag)
>  		use_editor = edit_flag;
>  
> @@ -1821,6 +1824,22 @@ int cmd_commit(int argc,
>  	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
>  					  builtin_commit_usage,
>  					  prefix, current_head, &s);
> +
> +	if (logfile && fixup_message && !strcmp(fixup_prefix, "amend")) {
> +		if (!strcmp(logfile, "-")) {
> +			if (isatty(0))
> +				fprintf(stderr, _("(reading log message from standard input)\n"));
> +			if (strbuf_read(&message, 0, 0) < 0)
> +				die_errno(_("could not read log from standard input"));
> +		} else {
> +			if (strbuf_read_file(&message, logfile, 0) < 0)
> +				die_errno(_("could not read log file '%s'"), logfile);
> +		}
> +		strbuf_complete_line(&message);
> +		have_option_m = 1;
> +		FREE_AND_NULL(logfile);
> +	}
> +

It is curious that for this new feature alone, but not the other
existing code paths, "-m" and "-F" options reads from file in the
new code here, instead of letting the existing code for "-F" to read
(which happens inside prepare_to_commit(), I presume?).

A potential problem of the above code is if we find something wrong
in message and complain later in the control flow, we have long lost
where the message came from, as the point of the above code is
exactly to pretend that "--fixup:amend/reword -F" message did *not*
come from a file with the "-F" option, but from the command line via
the "-m" option.

> +test_expect_success '--fixup=amend: with -m option' '
>  	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
> +	cat >expected <<-EOF &&

This comment is not about the added logic, but I notice that among
86 hits with string "expect" in this file in today's "master", only
14 hits are with string "expected", i.e., the prevalent name for the
"golden copy result" that is compared with the actula result (called
"actual") is "expect", not "expected".  Please do not make the
situation worse.

> -	test_cmp expect actual
> +	cat >expected <<-EOF &&

Ditto.

Re: [PATCH 1/1] commit: allow -m/-F with --fixup=amend: or reword:

[Phillip Wood]

On 18/05/2026 13:39, Junio C Hamano wrote:
> erik@cervined.in writes:
> 
>> From: Erik Cervin-Edin <erik@cervined.in>
> 
> The name on this overriding in-body From: line, and the name on
> Signed-off-by: line below, must match.  Please pick a name with or
> without hyphen and stick to it.
> 
>> --fixup=amend: and --fixup=reword: require an editor to supply the
>> replacement commit message. The -m and -F flags are rejected: -m is
>> caught by a die() in prepare_to_commit(), and -F is caught by
>> die_for_incompatible_opt4() which groups -F with --fixup as mutually
>> exclusive. This makes these modes unusable in non-interactive
>> workflows -- notably AI coding agents.
> 
> "Unusable" may be stronger than reality, as you can make creatie use
> of GIT_EDITOR to achieve what you want.  "awkward" or "poorly suited"
> would be more fitting.

Indeed

>> Plain --fixup (without amend: or reword:) continues to reject -F but
>> still accepts -m (even though it's practically a no-op).
> 
> Is it "practically a no-op"?  Wouldn't
> 
>     $ git commit --fixup <commit> -m "message body"
> 
> be useful to leave a message in the resulting commit, which is later
> to be squashed into the named <commit>?  Actually squashing with "fixup!"
> may lose the message supplied here, but wouldn't people use this
> facility to more easily identify what each of the fixups are about?

Yes, I find it quite useful to make a note of what the fixup is doing if 
I know I'm not going to squash it for a while.

> For the same reason, "-F" would be just as useful as "-m" in this context,
> and it feels a bit inconsistent to allow one while rejecting the other.

Yes, looking at the way the code is structured I wonder if these options 
were made incompatible to simplify the implementation, or maybe the 
implementation merely reflects those restrictions.

>> @@ -1821,6 +1824,22 @@ int cmd_commit(int argc,
>>   	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
>>   					  builtin_commit_usage,
>>   					  prefix, current_head, &s);
>> +
>> +	if (logfile && fixup_message && !strcmp(fixup_prefix, "amend")) {
>> +		if (!strcmp(logfile, "-")) {
>> +			if (isatty(0))
>> +				fprintf(stderr, _("(reading log message from standard input)\n"));
>> +			if (strbuf_read(&message, 0, 0) < 0)
>> +				die_errno(_("could not read log from standard input"));
>> +		} else {
>> +			if (strbuf_read_file(&message, logfile, 0) < 0)
>> +				die_errno(_("could not read log file '%s'"), logfile);
>> +		}
>> +		strbuf_complete_line(&message);
>> +		have_option_m = 1;
>> +		FREE_AND_NULL(logfile);
>> +	}
>> +
> 
> It is curious that for this new feature alone, but not the other
> existing code paths, "-m" and "-F" options reads from file in the
> new code here, instead of letting the existing code for "-F" to read
> (which happens inside prepare_to_commit(), I presume?).
> 
> A potential problem of the above code is if we find something wrong
> in message and complain later in the control flow, we have long lost
> where the message came from, as the point of the above code is
> exactly to pretend that "--fixup:amend/reword -F" message did *not*
> come from a file with the "-F" option, but from the command line via
> the "-m" option.

It is indeed unfortunate that we end up duplicating the code to read the 
logfile here. I wonder how hard it would be to refactor 
prepare_to_commit() so that it can accommodate "--fixup=amend:<commit> -F"

>> +test_expect_success '--fixup=amend: with -m option' '
>>   	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
>> +	cat >expected <<-EOF &&
> 
> This comment is not about the added logic, but I notice that among
> 86 hits with string "expect" in this file in today's "master", only
> 14 hits are with string "expected", i.e., the prevalent name for the
> "golden copy result" that is compared with the actula result (called
> "actual") is "expect", not "expected".  Please do not make the
> situation worse.

In this case it would be better to use

	test_commit_message HEAD <<-EOF
	amend! $(git log -1 --format=%s HEAD~)

	amend commit message
	EOF

and avoid creating actual and expect all together.

Thanks

Phillip

Re: [PATCH 1/1] commit: allow -m/-F with --fixup=amend: or reword:

[Erik Cervin Edin]

> > > --fixup=amend: and --fixup=reword: require an editor to supply the
> > > replacement commit message. The -m and -F flags are rejected: -m is
> > > caught by a die() in prepare_to_commit(), and -F is caught by
> > > die_for_incompatible_opt4() which groups -F with --fixup as mutually
> > > exclusive. This makes these modes unusable in non-interactive
> > > workflows -- notably AI coding agents.
> > 
> > "Unusable" may be stronger than reality, as you can make creatie use
> > of GIT_EDITOR to achieve what you want.  "awkward" or "poorly suited"
> > would be more fitting.
> 
> Indeed

Fair, "poorly suited" is more accurate. It's not impossible, just very
awkward.

> > > Plain --fixup (without amend: or reword:) continues to reject -F but
> > > still accepts -m (even though it's practically a no-op).
> > 
> > Is it "practically a no-op"?

No, I was mistaken. The message is kept until autosquash.

    The `-m` option may be used to supplement the log message of the
    created commit, but the additional commentary will be thrown away
    once the "fixup!" commit is squashed into _<commit>_ by `git rebase
    --autosquash`.

I was trying to fill in the gaps here on the intent of the pre-existing
behavior (to reject -F with --fixup) and I kind of assumed the message
was being discarded.

> > For the same reason, "-F" would be just as useful as "-m" in this context,
> > and it feels a bit inconsistent to allow one while rejecting the other.
> 
> Yes, looking at the way the code is structured I wonder if these options
> were made incompatible to simplify the implementation, or maybe the
> implementation merely reflects those restrictions.

I think it would. I kept the pre-existing behavior because I wasn't sure
if the rejection meant "Error. You are doing something that doesn't make
sense -- you probably meant to do something else" or "Sorry. What you're
trying to do is not supported"

A closer look at the original implementation 30884c9afc (commit: add
support for --fixup <commit> -m"<extra message>", 2017-12-22) makes it
clear the intent here is the latter:

    Those options could also support combining with -m, but given what
    they do I can't think of a good use-case for doing that, so I have not
    made the more invasive change of splitting up the logic in commit.c to
    first act on those, and then on -m options.

There is a case to not reject them, it was just deemed unnecessary
complex for something without a clear use-case.

In the ideal case, given that -m works (and does something useful), it's
reasonable to expect -F to do the same (for the same reasons as
--fixup=reword:.) Although, it's arguably less crucial in this
usecase. Given what its ephemeral nature, such a message is likely a
terse comment, -m "forgot to format" or similar.

I think it makes sense to allow -F for all --fixup variations, for
consistency. For the plain --fixup, -c/-C are probably less justifiable,
but -F mirroring -m seems worthwhile for consistency's sake in all
variations.

> > A potential problem of the above code is if we find something wrong
> > in message and complain later in the control flow
> > in message and complain later in the control flow, we have long lost
> > where the message came from, as the point of the above code is
> > exactly to pretend that "--fixup:amend/reword -F" message did *not*
> > come from a file with the "-F" option, but from the command line via
> > the "-m" option.

Now that you mention this, I guess a message on stdin can be arbitrarily
large, have null bytes and maybe some other oddities which the -m
would never have.

> I wonder how hard it would be to refactor prepare_to_commit()
> so that it can accommodate "--fixup=amend:<commit> -F"

I think this is doable.

> > > +test_expect_success '--fixup=amend: with -m option' '
> > >   	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
> > > +	cat >expected <<-EOF &&
> > 
> > This comment is not about the added logic, but I notice that among
> > 86 hits with string "expect" in this file in today's "master", only

> > 14 hits are with string "expected", i.e., the prevalent name for the
> > "golden copy result" that is compared with the actula result (called
> > "actual") is "expect", not "expected".  Please do not make the
> > situation worse.

Mea culpa. I overlooked this distinction.

> In this case it would be better to use
> 
> 	test_commit_message HEAD <<-EOF
> 	amend! $(git log -1 --format=%s HEAD~)
> 
> 	amend commit message
> 	EOF
> 
> and avoid creating actual and expect all together.

That would also work (except it has to be HEAD~2, since the reword
commit advances HEAD by one)

Thank you both for the review. I will reroll as a V2 taking your
suggestions into account.

- Erik

[PATCH v2 0/2] commit: allow -m/-F/-c/-C for all --fixup variations

[erik]

From: Erik Cervin-Edin <erik@cervined.in>

V1 was a single patch and only addressed -m/-F.  V2 makes two
substantive changes.

First, the refactor.  V1 added a special-cased file slurp in
cmd_commit() that pretended -F had been spelled -m.  Junio noted
this loses the "this came from a file" origin if anything goes
wrong later in the control flow [1], and Phillip suggested
refactoring prepare_to_commit() [2].  V2 does that: prepare_to_commit()
now consults fixup_message at each message-source branch and routes the
message origin flags through the same code path for all --fixup
variations.

Second, scope extended to -c/-C.  In the V1 thread I noted [3]
that -F made sense across all --fixup variants for consistency,
but that -c/-C felt "probably less justifiable" for plain --fixup.
Looking at it more while refactoring, the existing patchwork of
which flag works with which variant looked less and less like a
design and more like an accident -- once -F was threaded through
prepare_to_commit(), -c/-C fell out of the same path naturally
(blocked by the same die_for_incompatible_opt4() grouping that
caught -F).  This lives in its own patch (2/2) -- I won't object
if reviewers prefer to drop it or rolling it separately.

There is one wrinkle worth flagging on 2/2.  When -c/-C names a
source commit whose message starts with "amend! ",
prepare_amend_commit() strips that line -- the same stripping that
happens for a no-source --fixup=amend:<commit>.  This is independent
of which --fixup variant is being produced (the target); it depends
only on whether the -c/-C source is itself an amend!/reword! commit.
The upside is that

    git commit --fixup=amend:foo -C foo

and

    GIT_EDITOR=: git commit --fixup=amend:foo

produce the same commit.  If reviewers would rather -c/-C take the
source message verbatim, that's a small change and I'm open to it.

Smaller fixes from V1 review:

  * "unusable" softened to "poorly suited" in the rationale [1].

  * Dropped the incorrect claim that plain --fixup -m is
    "practically a no-op"

  * Adopted Phillip's suggestion to use test_commit_message in the
    new --fixup=amend: -m test, which also resolves the
    expected/expect golden-file naming Junio called out [1][2].

[1] https://lore.kernel.org/git/xmqqik8kc2nj.fsf@gitster.g/
[2] https://lore.kernel.org/git/ac6aaaca-2b7c-4892-ba93-0dc3e3c18ff7@gmail.com/
[3] https://lore.kernel.org/git/aguM7UIbAo19Zojv@mbp/

Erik Cervin-Edin (2):
  commit: allow -m/-F for all kinds of --fixup
  commit: allow -c/-C for all kinds of --fixup

 Documentation/git-commit.adoc             |  22 +++--
 builtin/commit.c                          |  41 ++++----
 t/t7500-commit-template-squash-signoff.sh | 114 +++++++++++++++++++---
 3 files changed, 133 insertions(+), 44 deletions(-)

Range-diff against v1:
1:  49e202f04b ! 1:  e9f07d49ee commit: allow -m/-F with --fixup=amend: or reword:
    @@ Metadata
     Author: Erik Cervin-Edin <erik@cervined.in>
     
      ## Commit message ##
    -    commit: allow -m/-F with --fixup=amend: or reword:
    +    commit: allow -m/-F for all kinds of --fixup
     
    -    commit: allow -m/-F with --fixup=amend: or reword:
    +    The ability to provide a commit message for git commit --fixup and its
    +    variations is limited:
     
    -    --fixup=amend: and --fixup=reword: require an editor to supply the
    -    replacement commit message. The -m and -F flags are rejected: -m is
    -    caught by a die() in prepare_to_commit(), and -F is caught by
    +      * Plain --fixup only allows using the -m flag
    +
    +      * The amend/reword --fixup variants only allow supplying the message
    +        using an editor
    +
    +    For amend/reword, the -m and -F flags are rejected: -m is caught by a
    +    die() in prepare_to_commit(), and -F is caught by
         die_for_incompatible_opt4() which groups -F with --fixup as mutually
    -    exclusive. This makes these modes unusable in non-interactive
    -    workflows -- notably AI coding agents.
    +    exclusive.  This makes these modes poorly suited for non-interactive
    +    workflows -- notably when using AI coding agents.
    +
    +    When support to use the -m option was introduced in [1] it was noted
    +    that there could be support for other options but at the time the use
    +    case was deemed too niche.  Later, when the amend suboption was
    +    introduced in [2] -m support for amend fixups was discussed but not
    +    pursued, and -F was already caught by the higher-layer incompatibility
    +    check grouping it with --fixup.
    +
    +    The rejections of these options hark back to when --fixup was
    +    introduced in [3] and as noted in [1] -- there's nothing inherently
    +    preventing support for them.  The current patchwork of which flags
    +    work with which --fixup variants has no strong logic to it, and
    +    allowing all of them simplifies both the code and the interface.
     
    -    When the amend suboption was introduced in 494d314a05 (commit: add
    -    amend suboption to --fixup to create amend! commit, 2021-03-15),
    -    -m support for amend fixups was discussed but not pursued, and -F
    -    was already caught by the higher-layer incompatibility check grouping
    -    it with --fixup.
    +    Allow -m and -F to supply the message body for all --fixup variations,
    +    mirroring the flow of a regular commit.  -c and -C, which are blocked
    +    by the same incompatibility check, are handled in the next commit.
     
    -    Allow -m and -F to supply the replacement message body for amend and
    -    reword fixups. When provided, bypass the editor and directly use the
    -    user's message as the body, replacing the original commit's message. For
    -    -F, the file contents are read into the message strbuf and then handled
    -    identically to -m.
    +    1. 30884c9afc (commit: add support for --fixup <commit> -m"<extra
    +       message>", 2017-12-22)
     
    -    Plain --fixup (without amend: or reword:) continues to reject -F but
    -    still accepts -m (even though it's practically a no-op).
    +    2. 494d314a05 (commit: add amend suboption to --fixup to create amend!
    +       commit, 2021-03-15)
     
    -    Signed-off-by: Erik Cervin Edin <erik@cervined.in>
    +    3. d71b8ba7c9 (commit: --fixup option for use with rebase --autosquash,
    +       2010-11-02)
    +
    +    Helped-by: Junio C Hamano <gitster@pobox.com>
    +    Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
    +    Signed-off-by: Erik Cervin-Edin <erik@cervined.in>
     
      ## Documentation/git-commit.adoc ##
    -@@ Documentation/git-commit.adoc: commit, but the additional commentary will be thrown away once the
    +@@ Documentation/git-commit.adoc: include::diff-context-options.adoc[]
    + The commit created by plain `--fixup=<commit>` has a title
    + composed of "fixup!" followed by the title of _<commit>_,
    + and is recognized specially by `git rebase --autosquash`. The `-m`
    +-option may be used to supplement the log message of the created
    +-commit, but the additional commentary will be thrown away once the
    +-"fixup!" commit is squashed into _<commit>_ by
    ++or `-F` option may be used to supplement the log message
    ++of the created commit, but the additional commentary will be thrown
    ++away once the "fixup!" commit is squashed into _<commit>_ by
    + `git rebase --autosquash`.
    + +
      The commit created by `--fixup=amend:<commit>` is similar but its
      title is instead prefixed with "amend!". The log message of
      _<commit>_ is copied into the log message of the "amend!" commit and
    @@ Documentation/git-commit.adoc: commit, but the additional commentary will be thr
     -log message to be empty unless `--allow-empty-message` is
     -specified.
     +opened in an editor so it can be refined. The replacement message may
    -+also be supplied directly using `-m` or `-F`, bypassing the need to open
    -+an editor. When `git rebase --autosquash` squashes the "amend!" commit
    -+into _<commit>_, the log message of _<commit>_ is replaced by the
    -+refined log message from the "amend!" commit. It is an error for the
    -+"amend!" commit's log message to be empty unless `--allow-empty-message`
    -+is specified.
    ++also be supplied directly using `-m` or `-F`, bypassing the
    ++need to open an editor. When `git rebase
    ++--autosquash` squashes the "amend!" commit into _<commit>_, the log
    ++message of _<commit>_ is replaced by the refined log message from the
    ++"amend!" commit. It is an error for the "amend!" commit's log message
    ++to be empty unless `--allow-empty-message` is specified.
      +
      `--fixup=reword:<commit>` is shorthand for `--fixup=amend:<commit>
       --only`. It creates an "amend!" commit with only a log message
     
      ## builtin/commit.c ##
    +@@ builtin/commit.c: 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";
    +-	} else if (logfile) {
    ++	} else if (logfile && !fixup_message) {
    + 		if (strbuf_read_file(&sb, logfile, 0) < 0)
    + 			die_errno(_("could not read log file '%s'"),
    + 				  logfile);
    + 		hook_arg1 = "message";
    +-	} else if (use_message) {
    ++	} else if (use_message && !fixup_message) {
    + 		const char *buffer;
    + 		buffer = strstr(use_message_buffer, "\n\n");
    + 		if (buffer)
     @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const char *prefix,
      		hook_arg1 = "message";
      
    @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const cha
     -		 * incompatible with all the forms of `--fixup` and
     -		 * have already errored out while parsing the `git commit`
     -		 * options.
    -+		 * `-m` (and `-F`, converted to `-m` earlier for
    -+		 * amend/reword) appends the message body here.
    -+		 * `-c`/`-C` are still incompatible with all forms
    -+		 * of `--fixup`.
    ++		 * 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 && !strcmp(fixup_prefix, "fixup"))
    ++		if (have_option_m) {
      			strbuf_addbuf(&sb, &message);
    - 
    - 		if (!strcmp(fixup_prefix, "amend")) {
    - 			if (have_option_m)
    +-
    +-		if (!strcmp(fixup_prefix, "amend")) {
    +-			if (have_option_m)
     -				die(_("options '%s' and '%s:%s' cannot be used together"), "-m", "--fixup", fixup_message);
    --			prepare_amend_commit(commit, &sb, &ctx);
    -+				strbuf_addbuf(&sb, &message);
    -+			else
    -+				prepare_amend_commit(commit, &sb, &ctx);
    ++		} 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);
    ++		} else if (!strcmp(fixup_prefix, "amend")) {
    + 			prepare_amend_commit(commit, &sb, &ctx);
      		}
      	} else if (!stat(git_path_merge_msg(the_repository), &statbuf)) {
    - 		size_t merge_msg_start;
     @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *argv[],
      	}
      	if (fixup_message && squash_message)
    @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *ar
      				  !!edit_message, "-c",
     -				  !!logfile, "-F",
      				  !!fixup_message, "--fixup");
    -+	die_for_incompatible_opt3(!!use_message, "-C",
    -+				  !!edit_message, "-c",
    -+				  !!logfile, "-F");
      	die_for_incompatible_opt4(have_option_m, "-m",
      				  !!edit_message, "-c",
    - 				  !!use_message, "-C",
    -@@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *argv[],
    - 		}
    - 	}
    - 
    -+	if (logfile && fixup_message && !strcmp(fixup_prefix, "fixup"))
    -+		die(_("options '%s' and '%s' cannot be used together"), "-F", "--fixup");
    -+
    - 	if (0 <= edit_flag)
    - 		use_editor = edit_flag;
    - 
    -@@ builtin/commit.c: int cmd_commit(int argc,
    - 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
    - 					  builtin_commit_usage,
    - 					  prefix, current_head, &s);
    -+
    -+	if (logfile && fixup_message && !strcmp(fixup_prefix, "amend")) {
    -+		if (!strcmp(logfile, "-")) {
    -+			if (isatty(0))
    -+				fprintf(stderr, _("(reading log message from standard input)\n"));
    -+			if (strbuf_read(&message, 0, 0) < 0)
    -+				die_errno(_("could not read log from standard input"));
    -+		} else {
    -+			if (strbuf_read_file(&message, logfile, 0) < 0)
    -+				die_errno(_("could not read log file '%s'"), logfile);
    -+		}
    -+		strbuf_complete_line(&message);
    -+		have_option_m = 1;
    -+		FREE_AND_NULL(logfile);
    -+	}
    -+
    - 	if (trailer_args.nr)
    - 		trailer_config_init();
    - 
     
      ## t/t7500-commit-template-squash-signoff.sh ##
     @@ t/t7500-commit-template-squash-signoff.sh: test_expect_success '--fixup=reword: ignores staged changes' '
    @@ t/t7500-commit-template-squash-signoff.sh: test_expect_success '--fixup=reword:
      '
      
     -test_expect_success '--fixup=reword: error out with -m option' '
    -+test_expect_success '--fixup=amend: 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 &&
    -+	cat >expect <<-EOF &&
    -+	amend! $(git log -1 --format=%s HEAD~)
    +-	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)
     +
    -+	amend commit message
    ++	reword commit message
     +	EOF
    -+	git commit --fixup=amend:HEAD~ -m "amend commit message" &&
    -+	get_commit_msg HEAD >actual &&
    - 	test_cmp expect actual
      '
      
     -test_expect_success '--fixup=amend: error out with -m option' '
    -+test_expect_success '--fixup=reword: 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 &&
    -+	cat >expect <<-EOF &&
    -+	amend! $(git log -1 --format=%s HEAD~)
    +-	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)
     +
    -+	reword commit message
    ++	amend commit message
     +	EOF
    -+	git commit --fixup=reword:HEAD~ -m "reword commit message" &&
    -+	get_commit_msg HEAD >actual &&
    - 	test_cmp expect actual
      '
      
    + test_expect_success 'consecutive amend! commits remove amend! line from commit msg body' '
     @@ t/t7500-commit-template-squash-signoff.sh: test_expect_success 'deny to create amend! commit if its commit msg body is empt
      	test_cmp expected actual
      '
      
    -+test_expect_success '--fixup=amend: -m with empty message aborts' '
    ++test_expect_success 'deny to create amend! commit if -m is empty' '
     +	commit_for_rebase_autosquash_setup &&
    -+	test_must_fail git commit --fixup=amend:HEAD~ -m "" 2>err &&
    -+	test_grep "empty commit message body" err
    ++	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' '
    @@ t/t7500-commit-template-squash-signoff.sh: test_expect_success '--fixup=reword:
     -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_expect_success '--fixup=reword: with -F option' '
    -+	commit_for_rebase_autosquash_setup &&
    -+	echo "message from file" >msgfile &&
    -+	cat >expect <<-EOF &&
    -+	amend! $(git log -1 --format=%s HEAD~)
    -+
    -+	message from file
    -+	EOF
    -+	git commit --fixup=reword:HEAD~ -F msgfile &&
    -+	get_commit_msg HEAD >actual &&
    - 	test_cmp expect actual
    - '
    - 
    -+test_expect_success '--fixup=amend: with -F option' '
    +-	test_cmp expect actual
    ++test_expect_success 'commit --fixup works with -F' '
     +	commit_for_rebase_autosquash_setup &&
    -+	echo "amend message from file" >msgfile &&
    -+	cat >expect <<-EOF &&
    -+	amend! $(git log -1 --format=%s HEAD~)
    ++	echo "message" >msgfile &&
    ++	git commit --fixup HEAD~ -F msgfile &&
    ++	test_commit_message HEAD <<-EOF
    ++	fixup! $(git log -1 --format=%s HEAD~2)
     +
    -+	amend message from file
    ++	message
     +	EOF
    -+	git commit --fixup=amend:HEAD~ -F msgfile &&
    -+	get_commit_msg HEAD >actual &&
    -+	test_cmp expect actual
     +'
     +
    -+test_expect_success '-F with plain --fixup still errors' '
    ++test_expect_success 'commit --fixup=reword: works with -F' '
     +	commit_for_rebase_autosquash_setup &&
    -+	echo "message" >msgfile &&
    -+	test_must_fail git commit --fixup HEAD~ -F msgfile 2>err &&
    -+	test_grep "cannot be used together" err
    -+'
    ++	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' '
    - 	commit_for_rebase_autosquash_setup &&
    - 	echo "log message from file" >msgfile &&
    +@@ t/t7500-commit-template-squash-signoff.sh: 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
-:  ---------- > 2:  b3fc743abf commit: allow -c/-C for all kinds of --fixup

base-commit: 208068f2d8ae29d7edaa245d9975b1b22ec65738
-- 
2.54.0.1014.g842965a2d5

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

[erik]

From: Erik Cervin-Edin <erik@cervined.in>

The ability to provide a commit message for git commit --fixup and its
variations is limited:

  * Plain --fixup only allows using the -m flag

  * The amend/reword --fixup variants only allow supplying the message
    using an editor

For amend/reword, the -m and -F flags are rejected: -m is caught by a
die() in prepare_to_commit(), and -F is caught by
die_for_incompatible_opt4() which groups -F with --fixup as mutually
exclusive.  This makes these modes poorly suited for non-interactive
workflows -- notably when using AI coding agents.

When support to use the -m option was introduced in [1] it was noted
that there could be support for other options but at the time the use
case was deemed too niche.  Later, when the amend suboption was
introduced in [2] -m support for amend fixups was discussed but not
pursued, and -F was already caught by the higher-layer incompatibility
check grouping it with --fixup.

The rejections of these options hark back to when --fixup was
introduced in [3] and as noted in [1] -- there's nothing inherently
preventing support for them.  The current patchwork of which flags
work with which --fixup variants has no strong logic to it, and
allowing all of them simplifies both the code and the interface.

Allow -m and -F to supply the message body for all --fixup variations,
mirroring the flow of a regular commit.  -c and -C, which are blocked
by the same incompatibility check, are handled in the next commit.

1. 30884c9afc (commit: add support for --fixup <commit> -m"<extra
   message>", 2017-12-22)

2. 494d314a05 (commit: add amend suboption to --fixup to create amend!
   commit, 2021-03-15)

3. d71b8ba7c9 (commit: --fixup option for use with rebase --autosquash,
   2010-11-02)

Helped-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Erik Cervin-Edin <erik@cervined.in>
---
 Documentation/git-commit.adoc             | 19 ++++----
 builtin/commit.c                          | 34 +++++++-------
 t/t7500-commit-template-squash-signoff.sh | 56 +++++++++++++++++------
 3 files changed, 69 insertions(+), 40 deletions(-)

diff --git a/Documentation/git-commit.adoc b/Documentation/git-commit.adoc
index 8329c1034b..61efd29e66 100644
--- a/Documentation/git-commit.adoc
+++ b/Documentation/git-commit.adoc
@@ -103,20 +103,21 @@ include::diff-context-options.adoc[]
 The commit created by plain `--fixup=<commit>` has a title
 composed of "fixup!" followed by the title of _<commit>_,
 and is recognized specially by `git rebase --autosquash`. The `-m`
-option may be used to supplement the log message of the created
-commit, but the additional commentary will be thrown away once the
-"fixup!" commit is squashed into _<commit>_ by
+or `-F` option may be used to supplement the log message
+of the created commit, but the additional commentary will be thrown
+away once the "fixup!" commit is squashed into _<commit>_ by
 `git rebase --autosquash`.
 +
 The commit created by `--fixup=amend:<commit>` is similar but its
 title is instead prefixed with "amend!". The log message of
 _<commit>_ is copied into the log message of the "amend!" commit and
-opened in an editor so it can be refined. When `git rebase
---autosquash` squashes the "amend!" commit into _<commit>_, the
-log message of _<commit>_ is replaced by the refined log message
-from the "amend!" commit. It is an error for the "amend!" commit's
-log message to be empty unless `--allow-empty-message` is
-specified.
+opened in an editor so it can be refined. The replacement message may
+also be supplied directly using `-m` or `-F`, bypassing the
+need to open an editor. When `git rebase
+--autosquash` squashes the "amend!" commit into _<commit>_, the log
+message of _<commit>_ is replaced by the refined log message from the
+"amend!" commit. It is an error for the "amend!" commit's log message
+to be empty unless `--allow-empty-message` is specified.
 +
 `--fixup=reword:<commit>` is shorthand for `--fixup=amend:<commit>
  --only`. It creates an "amend!" commit with only a log message
diff --git a/builtin/commit.c b/builtin/commit.c
index 28f6174503..3f1fca2919 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -804,18 +804,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";
-	} else if (logfile) {
+	} else if (logfile && !fixup_message) {
 		if (strbuf_read_file(&sb, logfile, 0) < 0)
 			die_errno(_("could not read log file '%s'"),
 				  logfile);
 		hook_arg1 = "message";
-	} else if (use_message) {
+	} else if (use_message && !fixup_message) {
 		const char *buffer;
 		buffer = strstr(use_message_buffer, "\n\n");
 		if (buffer)
@@ -837,20 +837,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);
+		} else if (!strcmp(fixup_prefix, "amend")) {
 			prepare_amend_commit(commit, &sb, &ctx);
 		}
 	} else if (!stat(git_path_merge_msg(the_repository), &statbuf)) {
@@ -1338,9 +1339,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 66aff8e097..01c7400136 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
-- 
2.54.0.1014.g842965a2d5

[PATCH v2 2/2] commit: allow -c/-C for all kinds of --fixup

[erik]

From: Erik Cervin-Edin <erik@cervined.in>

The previous commit allowed -m and -F for all --fixup variations.  The
-c/-C flags were blocked by the same higher-layer incompatibility check
that previously caught -F, namely die_for_incompatible_opt4() grouping
them with --fixup.

Drop --fixup from that check and route the resolved commit through
prepare_amend_commit() in the fixup path, mirroring the no-message-source
behaviour of --fixup=amend.  With this in place, -m/-F/-c/-C all behave
consistently across the plain, amend, and reword --fixup forms.

Signed-off-by: Erik Cervin-Edin <erik@cervined.in>
---
 Documentation/git-commit.adoc             |  9 ++--
 builtin/commit.c                          | 13 +++--
 t/t7500-commit-template-squash-signoff.sh | 60 +++++++++++++++++++++--
 3 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-commit.adoc b/Documentation/git-commit.adoc
index 61efd29e66..98c50a3be5 100644
--- a/Documentation/git-commit.adoc
+++ b/Documentation/git-commit.adoc
@@ -102,8 +102,8 @@ include::diff-context-options.adoc[]
 +
 The commit created by plain `--fixup=<commit>` has a title
 composed of "fixup!" followed by the title of _<commit>_,
-and is recognized specially by `git rebase --autosquash`. The `-m`
-or `-F` option may be used to supplement the log message
+and is recognized specially by `git rebase --autosquash`. The `-m`,
+`-F`, `-C`, or `-c` option may be used to supplement the log message
 of the created commit, but the additional commentary will be thrown
 away once the "fixup!" commit is squashed into _<commit>_ by
 `git rebase --autosquash`.
@@ -112,8 +112,9 @@ The commit created by `--fixup=amend:<commit>` is similar but its
 title is instead prefixed with "amend!". The log message of
 _<commit>_ is copied into the log message of the "amend!" commit and
 opened in an editor so it can be refined. The replacement message may
-also be supplied directly using `-m` or `-F`, bypassing the
-need to open an editor. When `git rebase
+also be supplied directly using `-m`, `-F`, or `-C`, bypassing the
+need to open an editor, or using `-c` to open the editor pre-populated
+with the referenced commit's message. When `git rebase
 --autosquash` squashes the "amend!" commit into _<commit>_, the log
 message of _<commit>_ is replaced by the refined log message from the
 "amend!" commit. It is an error for the "amend!" commit's log message
diff --git a/builtin/commit.c b/builtin/commit.c
index 3f1fca2919..fcf148eb21 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -837,9 +837,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		hook_arg1 = "message";
 
 		/*
-		 * Only `-m` and `-F` are handled here. `-c`/`-C` are
-		 * incompatible with --fixup and have already errored out
-		 * during option parsing.
+		 * `-m`, `-F`, `-C`, and `-c` provide the message body.
+		 * If none was given and this is an amend, use the target
+		 * commit's body instead.
 		 */
 		if (have_option_m) {
 			strbuf_addbuf(&sb, &message);
@@ -851,6 +851,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		} else if (logfile) {
 			if (strbuf_read_file(&sb, logfile, 0) < 0)
 				die_errno(_("could not read log file '%s'"), logfile);
+		} else if (use_message) {
+			struct commit *c = lookup_commit_reference_by_name(use_message);
+			if (!c)
+				die(_("could not lookup commit '%s'"), use_message);
+			prepare_amend_commit(c, &sb, &ctx);
 		} else if (!strcmp(fixup_prefix, "amend")) {
 			prepare_amend_commit(commit, &sb, &ctx);
 		}
@@ -1341,7 +1346,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
 	die_for_incompatible_opt3(!!use_message, "-C",
 				  !!edit_message, "-c",
-				  !!fixup_message, "--fixup");
+				  !!logfile, "-F");
 	die_for_incompatible_opt4(have_option_m, "-m",
 				  !!edit_message, "-c",
 				  !!use_message, "-C",
diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
index 01c7400136..48e1247d9e 100755
--- a/t/t7500-commit-template-squash-signoff.sh
+++ b/t/t7500-commit-template-squash-signoff.sh
@@ -492,6 +492,62 @@ test_expect_success 'commit --fixup works with -F' '
 	EOF
 '
 
+test_expect_success 'commit --fixup works with -C' '
+	commit_for_rebase_autosquash_setup &&
+	git commit --fixup HEAD~ -C HEAD &&
+	test_commit_message HEAD <<-EOF
+	fixup! $(git log -1 --format=%s HEAD~2)
+
+	$(get_commit_msg HEAD~)
+	EOF
+'
+
+test_expect_success 'commit --fixup=amend: works with -c' '
+	commit_for_rebase_autosquash_setup &&
+	test_set_editor : &&
+	git commit --fixup=amend:HEAD -c HEAD~ &&
+	test_commit_message HEAD <<-EOF
+	amend! intermediate commit
+
+	target message subject line
+
+	target message body line 1
+	target message body line 2
+	EOF
+'
+
+test_expect_success 'commit --fixup=amend:HEAD with -C HEAD and without have the same message' '
+	commit_for_rebase_autosquash_setup &&
+	start=$(git rev-parse HEAD) &&
+
+	git commit --fixup=amend:HEAD -C HEAD &&
+	git commit --fixup=amend:HEAD -C HEAD &&
+	git log -1 --pretty=%B >with-c &&
+
+	git reset --hard "$start" &&
+	test_set_editor : &&
+	git commit --fixup=amend:HEAD &&
+	git commit --fixup=amend:HEAD &&
+	git log -1 --pretty=%B >without-c &&
+
+	test_cmp with-c without-c
+'
+
+test_expect_success 'commit --fixup=amend: with -C copies full subject + body of squash commit' '
+	commit_for_rebase_autosquash_setup &&
+	git commit --squash HEAD~ -m "inner body" &&
+	echo "extra" >>foo &&
+	git add foo &&
+	git commit --fixup=amend:HEAD -C HEAD &&
+	test_commit_message HEAD <<-EOF
+	amend! squash! $(git log -1 --format=%s HEAD~3)
+
+	squash! $(git log -1 --format=%s HEAD~3)
+
+	inner body
+	EOF
+'
+
 test_expect_success 'commit --fixup=reword: works with -F' '
 	commit_for_rebase_autosquash_setup &&
 	echo "message from file" >msgfile &&
@@ -553,9 +609,7 @@ test_expect_success 'invalid message options when using --fixup' '
 	echo changes >>foo &&
 	echo "message" >log &&
 	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 --squash HEAD~2
 '
 
 cat >expected-template <<EOF
-- 
2.54.0.1014.g842965a2d5