From: Junio C Hamano Date: Fri, 26 Feb 2021 19:32:30 GMT Subject: Re: [PATCH v2 2/6] commit: add amend suboption to --fixup to create amend! commit Message-ID: In-Reply-To: > subject of head > body of head > > So, I am not sure about the other option to implement it ? Thaks, and OK. >> > + 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: >> 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: 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.