[PATCH v3 0/2] commit: allow -m/-F/-c/-C for all --fixup variations

[erik@cervined.in]

From: Erik Cervin-Edin <erik@cervined.in>

Changes since v2:

  * 2/2: Junio noted [1] that after dropping --fixup from the
    -C/-c/--fixup incompatibility check, what remained (-C, -c, -F)
    was already rejected by the neighbouring die_for_incompatible_opt4()
    that groups those options with -m.  The check is now removed
    entirely instead of being narrowed.

  * 2/2: reworded the log message to lead with why -c/-C are the odd
    ones out once 1/2 is in place, rather than with the check that
    happened to block them.

  * Rebased onto current master.  1/2 is unchanged apart from that.

Background
----------

Today only -m works with plain --fixup, and neither -m nor -F works
with --fixup=amend: or --fixup=reword:, which makes those modes
awkward to drive non-interactively.  1/2 threads -m and -F through
prepare_to_commit() for all --fixup variations.  2/2 does the same for
-c/-C, which were blocked by the same incompatibility check that
caught -F, so that all four message sources behave consistently across
the plain, amend, and reword forms.

2/2 lives in its own patch and I won't object if reviewers would
rather drop it or take it separately.

A note on one design decision in 2/2.  -c/-C go through
prepare_amend_commit(), so if the source commit is itself an "amend!"
commit, its subject line is dropped and only the body is copied.  This
is what a no-source --fixup=amend:<commit> already does.

I kept that on purpose.  Under --fixup the subject is always generated
by git, so whatever -C supplies ends up in the body, and the body of an
amend! commit is the message destined for its target.  Copying the
"amend! X" line along with it would leave that line in the final
message after autosquash.  It also means

    git commit --fixup=amend:foo -C foo

and

    GIT_EDITOR=: git commit --fixup=amend:foo

produce the same commit.

The helper only knows about "amend!" and not "fixup!" or "squash!".
That is pre-existing and I have left it alone here.

The v1 -> v2 changes (refactoring prepare_to_commit() instead of the
special-cased file slurp in cmd_commit(), extending scope to -c/-C,
and the smaller review fixes) are described in the v2 cover letter [2].

[1] https://lore.kernel.org/git/xmqq1pbkefh0.fsf@gitster.g/
[2] https://lore.kernel.org/git/cover.1779792311.git.erik@cervined.in/

Erik Cervin-Edin (2):
  commit: allow -m/-F for all kinds of --fixup
  commit: allow -c/-C for all kinds of --fixup

 Documentation/git-commit.adoc             |  22 +++--
 builtin/commit.c                          |  40 ++++----
 t/t7500-commit-template-squash-signoff.sh | 114 +++++++++++++++++++---
 3 files changed, 131 insertions(+), 45 deletions(-)

Range-diff against v2:
1:  e9f07d49ee = 1:  4581f97cb7 commit: allow -m/-F for all kinds of --fixup
2:  b3fc743abf ! 2:  39e408e147 commit: allow -c/-C for all kinds of --fixup
    @@ Metadata
      ## Commit message ##
         commit: allow -c/-C for all kinds of --fixup
     
    -    The previous commit allowed -m and -F for all --fixup variations.  The
    -    -c/-C flags were blocked by the same higher-layer incompatibility check
    -    that previously caught -F, namely die_for_incompatible_opt4() grouping
    -    them with --fixup.
    +    The fixup code path in prepare_to_commit() only knows how to take a
    +    message body from -m or -F.  Now that the previous commit allows -m and
    +    -F for all --fixup variations, -c/-C are the odd ones out.
     
    -    Drop --fixup from that check and route the resolved commit through
    -    prepare_amend_commit() in the fixup path, mirroring the no-message-source
    -    behaviour of --fixup=amend.  With this in place, -m/-F/-c/-C all behave
    -    consistently across the plain, amend, and reword --fixup forms.
    +    Teach the fixup path to look up the ref given to -c/-C and seed its
    +    message when preparing to amend the commit, mirroring what --fixup=amend
    +    does when no message source is given.  With this in place, -m/-F/-c/-C
    +    all behave consistently across the plain, amend, and reword --fixup
    +    forms.
     
    +    The incompatibility check grouping -C, -c, and --fixup can go away
    +    entirely, as -C and -c are already rejected together by the check that
    +    groups them with -m and -F.
    +
    +    Helped-by: Junio C Hamano <gitster@pobox.com>
         Signed-off-by: Erik Cervin-Edin <erik@cervined.in>
     
      ## Documentation/git-commit.adoc ##
    @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const cha
      			prepare_amend_commit(commit, &sb, &ctx);
      		}
     @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *argv[],
    + 	}
    + 	if (fixup_message && squash_message)
      		die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
    - 	die_for_incompatible_opt3(!!use_message, "-C",
    - 				  !!edit_message, "-c",
    +-	die_for_incompatible_opt3(!!use_message, "-C",
    +-				  !!edit_message, "-c",
     -				  !!fixup_message, "--fixup");
    -+				  !!logfile, "-F");
      	die_for_incompatible_opt4(have_option_m, "-m",
      				  !!edit_message, "-c",
      				  !!use_message, "-C",

base-commit: d38352cd43ab9745686d697872408bc3249a153f
-- 
2.55.0.770.g0473647dd8