All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: erik@cervined.in, git@vger.kernel.org
Cc: gitster@pobox.com
Subject: Re: [PATCH v3 2/2] commit: allow -c/-C for all kinds of --fixup
Date: Wed, 21 Oct 2026 16:10:49 +0100	[thread overview]
Message-ID: <de12af28-a64e-4472-8bd4-b65200772dbe@gmail.com> (raw)
In-Reply-To: <39e408e14718f533edc6ed2de6ff41ed961c6e05.1790082176.git.erik@cervined.in>

Hi Erik

On 22/09/2026 14:04, erik@cervined.in wrote:
> From: Erik Cervin-Edin <erik@cervined.in>
> 
> The fixup code path in prepare_to_commit() only knows how to take a
> message body from -m or -F.  Now that the previous commit allows -m and
> -F for all --fixup variations, -c/-C are the odd ones out.
> 
> Teach the fixup path to look up the ref given to -c/-C and seed its
> message when preparing to amend the commit, mirroring what --fixup=amend
> does when no message source is given.  With this in place, -m/-F/-c/-C
> all behave consistently across the plain, amend, and reword --fixup
> forms.
> 
> The incompatibility check grouping -C, -c, and --fixup can go away
> entirely, as -C and -c are already rejected together by the check that
> groups them with -m and -F.

I'm less convinced about there being much use for combining "--fixup" 
with "-C" but I don't see any reason to prevent it.

> @@ -863,6 +863,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) {

by the time we get here we've copied edit_message into use_message if 
"-c" was given so this covers "-C" and "-c".

> +			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);

> diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
> index 9b2ecb1356..de3221ea52 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 &&

It would be nice to see

	GIT_EDITOR=false git commit --fixup HEAD~ -C HEAD

so that we check the editor is not invoked.

> +	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 : &&

Most of the tests in this file correctly use a subshell when setting the 
editor so that they do not affect subsequent tests. It would be nice to 
check that the editor actually ran.

	GIT_EDITOR="echo edited >>" git commit ---fixup=amend:HEAD -c HEAD~ &&

Thanks

Phillip

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


  reply	other threads:[~2026-10-21 15:10 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18 11:22 [PATCH 0/1] commit: allow -m/-F with --fixup=amend: or reword: erik
2026-05-18 11:22 ` [PATCH 1/1] " erik
2026-05-18 12:39   ` Junio C Hamano
2026-05-18 15:27     ` Phillip Wood
2026-05-24 15:00       ` Erik Cervin Edin
2026-05-26 10:47 ` [PATCH v2 0/2] commit: allow -m/-F/-c/-C for all --fixup variations erik
2026-05-26 10:47   ` [PATCH v2 1/2] commit: allow -m/-F for all kinds of --fixup erik
2026-05-26 10:47   ` [PATCH v2 2/2] commit: allow -c/-C " erik
2026-08-26 21:31     ` Junio C Hamano
2026-09-04 18:47       ` Erik Cervin Edin
2026-09-22 13:04   ` [PATCH v3 0/2] commit: allow -m/-F/-c/-C for all --fixup variations erik
2026-09-22 13:04     ` [PATCH v3 1/2] commit: allow -m/-F for all kinds of --fixup erik
2026-10-21 15:11       ` Phillip Wood
2026-09-22 13:04     ` [PATCH v3 2/2] commit: allow -c/-C " erik
2026-10-21 15:10       ` Phillip Wood [this message]
2026-10-21 15:13     ` [PATCH v3 0/2] commit: allow -m/-F/-c/-C for all --fixup variations Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=de12af28-a64e-4472-8bd4-b65200772dbe@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=erik@cervined.in \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.