Re: [PATCH 1/1] commit: allow -m/-F with --fixup=amend: or reword:
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 18, 2026, 12:39 UTC
- Message-ID
- <xmqqik8kc2nj.fsf@gitster.g>
- In-Reply-To
- <20260518112225.73172-4-erik@cervined.in>
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.
Show 6 quoted lines
> --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.
Show 26 quoted lines
> 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.
Show 52 quoted lines
> - 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.
Show 6 quoted lines
> +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.