Re: [PATCH v3 3/9] cherry-pick: add --skip-empty option

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Pretty much what it says on the tin.

And a bit more, isn't it?

The --keep-redundant-commits option implies the --allow-empty option
and it was perfectly acceptable to give both.  By making sure that
only at most one of -k-r-d, -a-e or -s-e is given, this forbids that
usage.

"It is implied so there is no *need* to give it redundantly" is
different from "It is implied so you shouldn't give it redundantly".

Like 7 and 8 that adds --quiet/--skip, don't we want this also for
revert?

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  Documentation/git-cherry-pick.txt   |  3 +++
>  builtin/revert.c                    |  8 ++++++++
>  sequencer.c                         |  6 ++++++
>  sequencer.h                         |  1 +
>  t/t3508-cherry-pick-many-commits.sh | 13 +++++++++++++
>  5 files changed, 31 insertions(+)
>
> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
> index c205d23..fccd936 100644
> --- a/Documentation/git-cherry-pick.txt
> +++ b/Documentation/git-cherry-pick.txt
> @@ -129,6 +129,9 @@ effect to your index in a row.
>  	redundant commits are ignored.  This option overrides that behavior and
>  	creates an empty commit object.  Implies `--allow-empty`.
>  
> +--skip-empty::
> +	Instead of failing, skip commits that are or become empty.
> +
>  --strategy=<strategy>::
>  	Use the given merge strategy.  Should only be used once.
>  	See the MERGE STRATEGIES section in linkgit:git-merge[1]
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 0401fdb..5a8453d 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -118,6 +118,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
>  		OPT_END(),
>  		OPT_END(),
>  		OPT_END(),
> +		OPT_END(),
>  	};
>  
>  	if (opts->action == REPLAY_PICK) {
> @@ -127,6 +128,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
>  			OPT_BOOLEAN(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
>  			OPT_BOOLEAN(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
>  			OPT_BOOLEAN(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
> +			OPT_BOOLEAN(0, "skip-empty", &opts->skip_empty, N_("skip empty commits")),
>  			OPT_END(),
>  		};
>  		if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
> @@ -144,6 +146,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
>  				"--abort", rollback,
>  				NULL);
>  
> +	verify_opt_mutually_compatible(me,
> +				"--allow-empty", opts->allow_empty,
> +				"--skip-empty", opts->skip_empty,
> +				"--keep-redundant-commits", opts->keep_redundant_commits,
> +				NULL);
> +
>  	/* implies allow_empty */
>  	if (opts->keep_redundant_commits)
>  		opts->allow_empty = 1;
> diff --git a/sequencer.c b/sequencer.c
> index f7be7d8..d3c7d72 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -627,6 +627,12 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
>  		goto leave;
>  	}
>  
> +	if (opts->skip_empty && is_index_unchanged() == 1) {
> +		warning(_("skipping %s... %s"),
> +			find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
> +			msg.subject);
> +		goto leave;
> +	}
>  	allow = allow_empty(opts, commit);
>  	if (allow < 0) {
>  		res = allow;
> diff --git a/sequencer.h b/sequencer.h
> index 1fc22dc..3b04844 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -34,6 +34,7 @@ struct replay_opts {
>  	int allow_empty;
>  	int allow_empty_message;
>  	int keep_redundant_commits;
> +	int skip_empty;
>  
>  	int mainline;
>  
> diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
> index 19c99d7..3dc19c6 100755
> --- a/t/t3508-cherry-pick-many-commits.sh
> +++ b/t/t3508-cherry-pick-many-commits.sh
> @@ -187,4 +187,17 @@ test_expect_success 'cherry-pick --stdin works' '
>  	check_head_differs_from fourth
>  '
>  
> +test_expect_success 'cherry-pick skip empty' '
> +	git clean -fxd &&
> +	git checkout -b empty fourth &&
> +	git commit --allow-empty -m empty &&
> +	test_commit ontop &&
> +	git checkout -f master &&
> +	git reset --hard fourth &&
> +	git cherry-pick --skip-empty fourth..empty &&
> +	echo ontop > expected &&
> +	git log --format=%s fourth..HEAD > actual
> +	test_cmp expected actual
> +'
> +
>  test_done