From: Junio C Hamano Date: Thu, 25 Feb 2021 20:32:59 GMT Subject: Re: [PATCH v2 3/6] commit: add a reword suboption to --fixup Message-ID: In-Reply-To: <20210225100855.25530-4-charvi077@gmail.com> Charvi Mendiratta writes: > `git commit --fixup=reword:` creates an empty "amend!" commit > that will reword without changing its contents when it is > rebased with --autosquash. > > Apart from ignoring staged changes it works similarly to > `--fixup=amend:`. > > Mentored-by: Christian Couder > Mentored-by: Phillip Wood > Helped-by: Junio C Hamano > Signed-off-by: Charvi Mendiratta > --- > builtin/commit.c | 46 +++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 37 insertions(+), 9 deletions(-) > > diff --git a/builtin/commit.c b/builtin/commit.c > index 56ae15a762..82e77aa61d 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -1177,6 +1177,27 @@ static void finalize_deferred_config(struct wt_status *s) > s->ahead_behind_flags = AHEAD_BEHIND_FULL; > } > > +static void check_fixup_reword_options(int argc) { > + if (whence != FROM_COMMIT) { > + if (whence == FROM_MERGE) > + die(_("You are in the middle of a merge -- cannot reword.")); > + else if (is_from_cherry_pick(whence)) > + die(_("You are in the middle of a cherry-pick -- cannot reword.")); > + } > + if (argc) > + die(_("cannot combine reword option of --fixup with paths")); It would be easier if the user is told "foo" in the message when $ git commit --fixup=reword:HEAD~ -- foo (from your tests) is attempted, no? > + if (patch_interactive) > + die(_("cannot combine reword option of --fixup with --patch")); > + if (interactive) > + die(_("cannot combine reword option of --fixup with --interactive")); > + if (all) > + die(_("cannot combine reword option of --fixup with --all")); > + if (also) > + die(_("cannot combine reword option of --fixup with --include")); > + if (only) > + die(_("cannot combine reword option of --fixup with --only")); > +} > + > /* returns the length of intial segment of alpha characters only */ > static size_t get_alpha_len(char *fixup_message) { > const char alphas[] = "abcdefghijklmnopqrstuvwxyz"; > @@ -1261,18 +1282,25 @@ static int parse_and_validate_options(int argc, const char *argv[], > > if (fixup_message) { > /* > - * As `amend` suboption contains only alpha > - * character. So check if first non alpha > - * character in fixup_message is ':'. > + * As `amend`/`reword` suboptions contains only alpha > + * characters. So check if first non alpha character > + * in fixup_message is ':'. > */ > size_t len = get_alpha_len(fixup_message); > if (len && fixup_message[len] == ':') { > fixup_message[len] = '\0'; > fixup_commit = fixup_message + ++len; > - if (starts_with("amend", fixup_message)) > + if (starts_with("amend", fixup_message) || > + starts_with("reword", fixup_message)) { > fixup_prefix = "amend"; > - else > + if (*fixup_message == 'r') { > + check_fixup_reword_options(argc); > + allow_empty = 1; > + only = 1; OK. We make sure that there is no pathspec and then by giving only==1, we ignore any difference that already may exist between HEAD and the index. IOW edit somefile git add somefile git commit --fixup=reword: will still record the same tree as HEAD, without affected by the addition of somefile to the index that is done before. Good. > + } > + } else { > die(_("unknown option: --fixup=%s:%s"), fixup_message, fixup_commit); > + } > } else { > fixup_commit = fixup_message; > fixup_prefix = "fixup"; > @@ -1558,11 +1586,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > OPT_STRING('c', "reedit-message", &edit_message, N_("commit"), N_("reuse and edit message from specified commit")), > OPT_STRING('C', "reuse-message", &use_message, N_("commit"), N_("reuse message from specified commit")), > /* > - * TRANSLATORS: please do not translate [amend:] > - * Here "amend" is an option to the --fixup command > - * line flag, that creates amend! commit. > + * TRANSLATORS: please do not translate [(amend|reword):] > + * Here "amend" and "reword" are options to the --fixup > + * command line flag, that creates amend! commit. I am not sure this comment is all that helpful to the translaters. If they are not allowed to translate part, telling them what that part means does not help them very much. Leave "[(amend|reword):]" as-is, and only translate . would be more direct without distracting them with useless piece of information, no? > */ > - OPT_STRING(0, "fixup", &fixup_message, N_("[amend:]commit"), N_("use autosquash formatted message to fixup or amend specified commit")), > + OPT_STRING(0, "fixup", &fixup_message, N_("[(amend|reword):]commit"), N_("use autosquash formatted message to fixup or amend/reword specified commit")), > OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")), > OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")), > OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),