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

From: Junio C Hamano <[email-masked]>
Date: Thu, 23 Feb 2023 14:35:49 -0800

"Phillip Wood via GitGitGadget" <[email-masked]> writes:

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

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