Re: [PATCH v3 2/6] commit: add amend suboption to --fixup to create amend! commit
- From
Charvi Mendiratta <charvi077@gmail.com>
- Date
- Mar 3, 2021, 19:21 UTC
- Message-ID
- <CAPSFM5eAdfAD+TeLn8f4+1x38g_6r7T=DuyPJf2f-828pwqgyQ@mail.gmail.com>
- In-Reply-To
- <CAPig+cQcjUzXpW1OBCiEC3Sr13uyeesqK49KWmgnY1iHNnS2Rw@mail.gmail.com>
On Wed, 3 Mar 2021 at 13:27, Eric Sunshine <sunshine@sunshineco.com> wrote:
>
[...]
Show 14 quoted lines
> > 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/
Show 12 quoted lines
>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 .