From: Charvi Mendiratta Date: Wed, 03 Mar 2021 19:21:57 GMT Subject: Re: [PATCH v3 2/6] commit: add amend suboption to --fixup to create amend! commit Message-ID: In-Reply-To: On Wed, 3 Mar 2021 at 13:27, Eric Sunshine wrote: > [...] > > I am not sure about this because we used the starts_with() so that it can > > support the _any_ prefix of `amend` or `reword` i.e to make all below > > like combinations possible : > > --fixup=a:HEAD~2 > > --fixup=am:HEAD~2 > > > > So, I am not sure if we need to replace it with !strcmp and work for > > the specified prefix only ? > > Hmm, I see. I didn't follow whatever discussion led to the decision to > use this sort of prefix matching, but I have to wonder if it is a good > idea. Was the idea that it behave similarly to sequencer commands in > `git rebase --interactive` which are often abbreviated to a single > letter? Yes, this is also true. Also, same is discussed as here: https://lore.kernel.org/git/CAPSFM5cEnex1xaBy5ia_xNFDNzt5_Y=W-6TB9d9yW_AiPAKxDg@mail.gmail.com/ >I personally would feel much more comfortable requiring a > full-word match for `amend` and `reword` at initial implementation. > That leaves the door open to later loosening it to do prefix-matching > if enough people request such a feature, whereas starting with > prefix-matching closes that door since we can never later tighten it > to require full words. > > Anyhow, if the decision is to keep this behavior, then it almost > certainly deserves an in-code comment explaining the sort of > prefix-matching it's doing since it's otherwise too easy for readers > to be fooled as Junio and I were by not noticing that you had reversed > the arguments to starts_with(). Okay, I will add the comments to it .