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

subject of head
body of head

So, I am not sure about the other option to implement it ?

Thaks, and OK.

quoted

quoted

+                     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.

On Sat, 27 Feb 2021 at 01:02, Junio C Hamano [off-list ref] wrote:
[...]

quoted

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.

I agree, earlier I even asked myself why only `-m`, but this was the
only option that was allowing to write/append commit message in `
--fixup`. And I agree to add a comment, to make it more clear.

Thanks and Regards,
Charvi