Jeff King <[email protected]> writes:
> Here it is in patch form, with an updated commit message that hopefully
> explains the rationale a bit better.
>
> -- >8 --
> Subject: [PATCH] commit: tweak empty cherry pick advice for sequencer
>
> When we refuse to make an empty commit, we check whether we
> are in a cherry-pick in order to give better advice on how
> to proceed. We instruct the user to repeat the commit with
> "--allow-empty" to force the commit, or to use "git reset"
> to skip it and abort the cherry-pick.
>
> In the case of a single cherry-pick, the distinction between
> skipping and aborting is not important, as there is no more
> work to be done afterwards. When we are using the sequencer
> to cherry pick a series of commits, though, the instruction
> is confusing: does it skip this commit, or does it abort the
> rest of the cherry-pick?
>
> It does skip, after which the user can continue the
> cherry-pick. This is the right thing to be advising the user
> to do, but let's make it more clear what will happen, both
> by using the word "skip", and by mentioning that the rest of
> the sequence can be continued via "cherry-pick --continue"
> (whether we skip or take the commit).
>
> Noticed-by: Ramkumar Ramachandra <[email protected]>
> Helped-by: Jonathan Nieder <[email protected]>
> Signed-off-by: Jeff King <[email protected]>
> ---
> builtin/commit.c | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index e47f100..73fafe2 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -63,8 +63,18 @@ N_("The previous cherry-pick is now empty, possibly due to
> conflict resolution.\
> "If you wish to commit it anyway, use:\n"
> "\n"
> " git commit --allow-empty\n"
> +"\n");
> +
> +static const char empty_cherry_pick_advice_single[] =
> +N_("Otherwise, please use 'git reset'\n");
> +
> +static const char empty_cherry_pick_advice_multi[] =
> +N_("If you wish to skip this commit, use:\n"
> "\n"
> -"Otherwise, please use 'git reset'\n");
> +" git reset\n"
> +"\n"
> +"Then \"git cherry-pick --continue\" will resume cherry-picking\n"
> +"the remaining commits.\n");
>
> static const char *use_message_buffer;
> static const char commit_editmsg[] = "COMMIT_EDITMSG";
> @@ -107,6 +117,7 @@ static enum commit_whence whence;
> static const char *cleanup_arg;
>
> static enum commit_whence whence;
> +static int sequencer_in_use;
> static int use_editor = 1, include_status = 1;
> static int show_ignored_in_status, have_option_m;
> static const char *only_include_assumed;
> @@ -141,8 +152,11 @@ static void determine_whence(struct wt_status *s)
> {
> if (file_exists(git_path("MERGE_HEAD")))
> whence = FROM_MERGE;
> - else if (file_exists(git_path("CHERRY_PICK_HEAD")))
> + else if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
> whence = FROM_CHERRY_PICK;
> + if (file_exists(git_path("sequencer")))
> + sequencer_in_use = 1;
Should this be
sequencer_in_use = file_exists(...)
so readers do not have to wonder what the variable is initialized
to?
Other than that, looks reasonable to me. Thanks.
> + }
> else
> whence = FROM_COMMIT;
> if (s)
> @@ -808,8 +822,13 @@ static int prepare_to_commit(const char *index_file,
> const char *prefix,
> run_status(stdout, index_file, prefix, 0, s);
> if (amend)
> fputs(_(empty_amend_advice), stderr);
> - else if (whence == FROM_CHERRY_PICK)
> + else if (whence == FROM_CHERRY_PICK) {
> fputs(_(empty_cherry_pick_advice), stderr);
> + if (!sequencer_in_use)
> + fputs(_(empty_cherry_pick_advice_single),
> stderr);
> + else
> + fputs(_(empty_cherry_pick_advice_multi),
> stderr);
> + }
> return 0;
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
