Re: [PATCH v3 2/6] commit: add amend suboption to --fixup to create amend! commit

On Tue, 2 Mar 2021 at 04:17, Eric Sunshine [off-list ref] wrote:

On Mon, Mar 1, 2021 at 5:32 PM Junio C Hamano [off-list ref] wrote:

quoted

Eric Sunshine [off-list ref] writes:

quoted

    if (skip_prefix(msg, "amend:", &arg) ||
        skip_prefix(msg, "reword:", &arg)) {
        ...
    }

You still need to compute "len" because you'd want to tell between
--fixup="HEAD^{/^area: string}" and --fixup=bogus:HEAD (the latter
would want to say "no such variant 'bogus' for --fixup", but the
colon in the former is not the end of the name of variant.

I see what you mean. I vaguely recall quickly scanning over that
earlier discussion about ":" being otherwise legitimate when embedded
in the argument as you demonstrate, but didn't think about it when
reading this code. Perhaps the comment which this code adds:

    * As `amend` suboption contains only alpha
    * character. So check if first non alpha
    * character in fixup_message is ':'.

could be extended a bit to mention that briefly since, without it, the
significance of "alpha-only characters followed by colon" is not
immediately obvious.

Okay, I will reword it and make it more clear regarding the main
reason for choosing this way.