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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 26, 2021, 19:32 UTC
Message-ID: <xmqqczwmsjzl.fsf@gitster.g>
In-Reply-To: <CAPSFM5eKnqOhDrZWEd-c_sDNMbQkqtMwpFM0zT-gOVHvMtcZnQ@mail.gmail.com>

> subject of head
> body of head
>
> So, I am not sure about the other option to implement it ?

Thaks, and OK.

Show 13 quoted lines

>> > +                     if (have_option_m)
>> > +                             die(_("cannot combine -m with --fixup:%s"), fixup_message);
>> > +                     else
>> > +                             prepare_amend_commit(commit, &sb, &ctx);
>>
>> Hmph, why is -m so special?  Should we allow --fixup=amend:<cmd>
>> with -F (or -c/-C for that matter), or are these other options
>> caught at a lot higher layer already and we do not have to check
>> them here?
>
> yes, those options are caught earlier and give the error as below:
> "Only one of -c/-C/-F/--fixup can be used."
> and only `-m` is checked over here.

And the reason why -m cannot be checked early is because we do not recognize which kind of "fixup" we are doing when "only one of -c/-C/-F/--fixup" check is made before this function is called?

OK. I wonder if we can tell which kind of fixup we are doing much earlier, though. Then we could extend it to say "Only one of -c/-C/-F/-m/--fixup=amend:<commit> can be used", etc., and we do not have to have this "only -m is checked here, everything else is checked earlier" curiosity. But I do not know if such a change is necessarily an improvement. I guess a better "fix" would probably be to add a comment to this function where it only checks for "-m" and tell readers why -c/-C/-F do not have to be checked here.

Thanks.