From: Eric Sunshine Date: Mon, 01 Mar 2021 22:47:38 GMT Subject: Re: [PATCH v3 2/6] commit: add amend suboption to --fixup to create amend! commit Message-ID: In-Reply-To: On Mon, Mar 1, 2021 at 5:32 PM Junio C Hamano wrote: > Eric Sunshine writes: > > 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.