Re: [PATCH 2/2] rebase -i: fix parsing of "fixup -C<commit>"

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 23, 2023, 22:35 UTC
Message-ID: <xmqq1qmg6md6.fsf@gitster.g>
In-Reply-To: <d670e284c175db67337ca7ce774ecd9afb089736.1677185702.git.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 6 quoted lines

>  	if (item->command == TODO_FIXUP) {
> -		if (skip_prefix(bol, "-C", &bol) &&
> -		   (*bol == ' ' || *bol == '\t')) {
> +		if (skip_prefix(bol, "-C", &bol)) {
>  			bol += strspn(bol, " \t");
>  			item->flags |= TODO_REPLACE_FIXUP_MSG;

OK. An explicit check followed by strspn() is an odd way to write this even if it meant to require at least one whitespace, but I agree that this one probably did not even mean to require a whitespace there, and the updated code looks much easier to read.

Show 13 quoted lines

> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> index 1d2f0429aea..7ca5b918f04 100644
> --- a/t/lib-rebase.sh
> +++ b/t/lib-rebase.sh
> ...
> +test_expect_success 'fixup -[Cc]<commit> works' '
> +	test_when_finished "test_might_fail git rebase --abort" &&
> +	cat >todo <<-\EOF &&
> +	pick A
> +	fixup -CA1
> +	pick B
> +	fixup -cA2
> +	EOF

By the way, this is much easier to follow, than the todo file written in the ugly FAKE_LINES language, to see what is being tested.

Will queue.  Thanks.