[PATCH 4/5] cherry-pick: allow skipping only redundant commits

[Giuseppe Bilotta <giuseppe.bilotta@gmail.com>]

This allows the preservation of originally empty commits with the
combination of flags --allow-empty --skip-redundant-commits.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 Documentation/git-cherry-pick.txt |  8 ++++-
 builtin/revert.c                  | 18 +++++++++++-
 sequencer.c                       | 62 ++++++++++++++++++++++++++++++---------
 sequencer.h                       |  1 +
 4 files changed, 73 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index ffced816d6..147e0cde0c 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -138,9 +138,15 @@ effect to your index in a row.
 	examine the commit. This option overrides that behavior and
 	creates an empty commit object.  Implies `--allow-empty`.
 
+--skip-redundant-commits::
+	Redundant commits will be skipped altogether. This does not
+	influence commits that were originally empty (see
+	`--allow-empty` and `--skip-empty`).
+
 --skip-empty::
 	This option causes empty and redundant cherry-picked commits to
-	be skipped without requesting the user intervention.
+	be skipped without requesting the user intervention. Implies
+	`--skip-redundant-commits`.
 
 --strategy=<strategy>::
 	Use the given merge strategy.  Should only be used once.
diff --git a/builtin/revert.c b/builtin/revert.c
index ffdd367f99..aca8a1d9d0 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -102,7 +102,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
 			OPT_BOOL(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
 			OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
 			OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
-			OPT_BOOL(0, "skip-empty", &opts->skip_empty, N_("skip redundant, empty commits")),
+			OPT_BOOL(0, "skip-empty", &opts->skip_empty, N_("skip both redundant and initially empty commits")),
+			OPT_BOOL(0, "skip-redundant-commits", &opts->skip_redundant_commits, N_("skip redundant commits")),
 			OPT_END(),
 		};
 		options = parse_options_concat(options, cp_extra);
@@ -115,6 +116,9 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
 	/* implies allow_empty */
 	if (opts->keep_redundant_commits)
 		opts->allow_empty = 1;
+	/* implies skip_redundant_commits */
+	if (opts->skip_empty)
+		opts->skip_redundant_commits = 1;
 
 	/* Check for incompatible command line arguments */
 	if (cmd) {
@@ -147,6 +151,18 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
 				"--edit", opts->edit,
 				NULL);
 
+	if (opts->keep_redundant_commits)
+		verify_opt_compatible(me, "--keep-redundant-commits",
+				"--skip-empty", opts->skip_empty,
+				"--skip-redundant-commits", opts->skip_redundant_commits,
+				NULL);
+
+	if (opts->keep_redundant_commits)
+		verify_opt_compatible(me, "--skip-empty",
+				"--allow-empty", opts->allow_empty,
+				"--keep-redundant-commits", opts->keep_redundant_commits,
+				NULL);
+
 	if (cmd) {
 		opts->revs = NULL;
 	} else {
diff --git a/sequencer.c b/sequencer.c
index 9c01310162..333d9112de 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -563,40 +563,70 @@ static int allow_or_skip_empty(struct replay_opts *opts, struct commit *commit)
 {
 	int index_unchanged, empty_commit;
 
-	/*
-	 * Four cases:
+	/* We have four options:
 	 *
-	 * (1) we do not allow empty at all and error out;
+	 * --allow-empty (AE)
+	 * --keep-redundant-commits (KR)
+	 * --skip-empty (SE)
+	 * --skip-redundant-commits (SR)
 	 *
-	 * (2) we skip empty commits altogether;
+	 * Additionally, if KR, then AE. And if SE, then SR.
+	 * 
+	 * We have three possible states:
+	 * Not Empty (NE)
+	 * Originally Empty (OE)
+	 * made REdundant (RE) (originally not empty)
 	 *
-	 * (3) we allow ones that were initially empty, but
-	 * forbid the ones that become empty;
+	 * NE always gets committed. The other two depend on the combination
+	 * of flags.
 	 *
-	 * (4) we allow both.
+	 *              OE outcome | RE outcome | AE  KR  SE  SR
+	 *     Case 0:  0 (error)    0 (error)     0   0   0   0
+	 *     Case 1:  1 (allow)    0 (error)     1   0   0   0
+	 * N/A Case 2:  2 (skip)     0 (error)     0   0   1   0
+	 * N/A Case 3:  0 (error)    1 (keep)      0   1   0   0
+	 *     Case 4:  1 (allow)    1 (keep)      1   1   0   0
+	 * N/A Case 5:  2 (skip)     1 (keep)      0   1   1   0
+	 *     Case 6:  0 (error)    2 (skip)      0   0   0   1
+	 *     Case 7:  1 (allow)    2 (skip)      1   0   0   1
+	 *     Case 8:  2 (skip )    2 (skip)      0   0   1   1
+	 *
+	 * TODO should we allow Case 2? If so, how?
 	 */
-	if (!opts->allow_empty && !opts->skip_empty)
+
+	/* Case 0 */
+	if (!opts->allow_empty && !opts->skip_redundant_commits)
 		return 0; /* let "git commit" barf as necessary */
 
 	index_unchanged = is_index_unchanged();
 	if (index_unchanged < 0)
 		return index_unchanged;
+
 	if (!index_unchanged)
 		return 0; /* we do not have to say --allow-empty */
 
-	if (opts->skip_empty)
-		return 2;
+	/* Here we know that the commit is either OE or RE */
 
+	/* Case 4, we don't care, result is 'allow' for both cases */
 	if (opts->keep_redundant_commits)
 		return 1;
 
+	/* Case 8, we don't care, result is 'skip' for both cases */
+	if (opts->skip_empty)
+		return 2;
+
+	/* Now we must differentiate between OE and RE,
+	 * for Case 1, 6, 7 */
 	empty_commit = is_original_commit_empty(commit);
 	if (empty_commit < 0)
 		return empty_commit;
-	if (!empty_commit)
-		return 0;
-	else
-		return 1;
+
+	/* An OE will return 1 if AE, 0 otherwise */
+	if (empty_commit)
+		return opts->allow_empty;
+
+	/* An RE will return 2 if SR, 0 otherwise */
+	return 2*opts->skip_redundant_commits;
 }
 
 enum todo_command {
@@ -1009,6 +1039,8 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
 		opts->keep_redundant_commits = git_config_bool_or_int(key, value, &error_flag);
 	else if (!strcmp(key, "options.skip-empty"))
 		opts->skip_empty = git_config_bool_or_int(key, value, &error_flag);
+	else if (!strcmp(key, "options.skip-redundant-commits"))
+		opts->skip_redundant_commits = git_config_bool_or_int(key, value, &error_flag);
 	else if (!strcmp(key, "options.mainline"))
 		opts->mainline = git_config_int(key, value);
 	else if (!strcmp(key, "options.gpg-sign"))
@@ -1267,6 +1299,8 @@ static int save_opts(struct replay_opts *opts)
 		res |= git_config_set_in_file_gently(opts_file, "options.keep-redundant-commits", "true");
 	if (opts->skip_empty)
 		res |= git_config_set_in_file_gently(opts_file, "options.skip-empty", "true");
+	if (opts->skip_redundant_commits)
+		res |= git_config_set_in_file_gently(opts_file, "options.skip-redundant-commits", "true");
 	if (opts->mainline) {
 		struct strbuf buf = STRBUF_INIT;
 		strbuf_addf(&buf, "%d", opts->mainline);
diff --git a/sequencer.h b/sequencer.h
index c747cfcfc7..f8b8bd0063 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -24,6 +24,7 @@ struct replay_opts {
 	int allow_empty_message;
 	int keep_redundant_commits;
 	int skip_empty;
+	int skip_redundant_commits;;
 
 	int mainline;
 
-- 
2.11.0.616.gd72966cf44.dirty