From: Phillip Wood <phillip.wood123@gmail.com>
Date: Wed, 12 Jun 2019 15:16:43 GMT
Subject: Re: [GSoC][PATCH v2 3/3] cherry-pick/revert: advise using --skip
Message-ID: <3591ee0f-b302-23c8-dda5-00ff650887b8@gmail.com>
In-Reply-To: <20190611073152.12214-4-rohit.ashiwal265@gmail.com>

Hi Rohit

On 11/06/2019 08:31, Rohit Ashiwal wrote:
> The previous commit introduced a --skip flag for cherry-pick and
> revert. Update the advice messages, to tell users about this less
> cumbersome way of skipping commits. Also add tests to ensure
> everything is working fine.
> 
> Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
> ---
> changes since last revision:
>      - selectively advice --skip
>      - add new test
> 
>   builtin/commit.c                | 13 ++++++++-----
>   sequencer.c                     |  7 +++++--
>   t/t3510-cherry-pick-sequence.sh | 20 ++++++++++++++++++++
>   3 files changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 1c9e8e2228..1f47c51bdc 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -60,15 +60,18 @@ N_("The previous cherry-pick is now empty, possibly due to conflict resolution.\
>   "\n");
>   
>   static const char empty_cherry_pick_advice_single[] =
> -N_("Otherwise, please use 'git reset'\n");
> +N_("Otherwise, please use 'git cherry-pick --skip'\n");
>   
>   static const char empty_cherry_pick_advice_multi[] =
> -N_("If you wish to skip this commit, use:\n"
> +N_("and then use:\n"
>   "\n"
> -"    git reset\n"
> +"    git cherry-pick --continue\n"
>   "\n"
> -"Then \"git cherry-pick --continue\" will resume cherry-picking\n"
> -"the remaining commits.\n");
> +"to resume cherry-picking the remaining commits.\n"
> +"If you wish to skip this commit, use:\n"
> +"\n"
> +"    git cherry-pick --skip\n"
> +"\n");
>   
>   static const char *color_status_slots[] = {
>   	[WT_STATUS_HEADER]	  = "header",
> diff --git a/sequencer.c b/sequencer.c
> index 93284cd7dd..ecf4be7e15 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2661,10 +2661,13 @@ static int create_seq_dir(struct repository *r)
>   			error(_("a %s is already in progress"),
>   				 action == REPLAY_REVERT ?
>   				 "revert" : "cherry-pick");
> -			advise(_("try \"git %s (--continue | "
> +			advise(_("try \"git %s (--continue | %s"
>   				 "--quit | --abort)\""),
>   				 action == REPLAY_REVERT ?
> -				 "revert" : "cherry-pick");
> +				 "revert" : "cherry-pick",
> +				 !file_exists(git_path_revert_head(r)) ?
> +				 !file_exists(git_path_cherry_pick_head(r)) ? ""
> +				 : "--skip | " : "--skip | ");

This could be simplified as
	(file_exists(git_path_revert_head(r) ||
	file_exists(git_path_cherry_pick_head(r)) ?
	"--skip | " ""

Best Wishes

Phillip

>   			return -1;
>   		default:
>   			BUG(_("the control must not reach here"));
> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
> index 6c1903a735..f298f02cd0 100755
> --- a/t/t3510-cherry-pick-sequence.sh
> +++ b/t/t3510-cherry-pick-sequence.sh
> @@ -172,6 +172,26 @@ test_expect_success 'check advice when we move HEAD by committing' '
>   	test_cmp expect advice
>   '
>   
> +test_expect_success 'selectively advise --skip while launching another sequence' '
> +	pristine_detach initial &&
> +	cat >expect <<-EOF &&
> +	error: a cherry-pick is already in progress
> +	hint: try "git cherry-pick (--continue | --skip | --quit | --abort)"
> +	fatal: cherry-pick failed
> +	EOF
> +	test_must_fail git cherry-pick picked..yetanotherpick &&
> +	test_must_fail git cherry-pick picked..yetanotherpick 2>advice && > +	test_cmp expect advice &&
> +	cat >expect <<-EOF &&
> +	error: a cherry-pick is already in progress
> +	hint: try "git cherry-pick (--continue | --quit | --abort)"
> +	fatal: cherry-pick failed
> +	EOF
> +	git reset --merge &&
> +	test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
> +	test_cmp expect advice
> +'
> +
>   test_expect_success 'allow skipping commit but not abort for a new history' '
>   	pristine_detach initial &&
>   	cat >expect <<-EOF &&
>