git am or rebase advise the user to use `git (am | rebase) --skip` to
skip the commit. cherry-pick and revert also have this concept of
skipping commits but they advise the user to use `git reset` (or in
case of a patch which had conflicts, `git reset --merge`) which on the
user's part is annoying and sometimes confusing. Add a `--skip` option
to make these commands more consistent.

In the next commit, we will change the advice messages hence finishing
the process of teaching revert and cherry-pick "how to skip commits".

Signed-off-by: Rohit Ashiwal <rohit.ashiwal...@gmail.com>
---
 Documentation/git-cherry-pick.txt |  4 +-
 Documentation/git-revert.txt      |  4 +-
 Documentation/sequencer.txt       |  4 ++
 builtin/revert.c                  |  5 +++
 sequencer.c                       | 23 +++++++++++
 sequencer.h                       |  1 +
 t/t3510-cherry-pick-sequence.sh   | 63 +++++++++++++++++++++++++++++++
 7 files changed, 98 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt 
b/Documentation/git-cherry-pick.txt
index 754b16ce0c..955880ab88 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -10,9 +10,7 @@ SYNOPSIS
 [verse]
 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff]
                  [-S[<keyid>]] <commit>...
-'git cherry-pick' --continue
-'git cherry-pick' --quit
-'git cherry-pick' --abort
+'git cherry-pick' --continue | --skip | --abort | --quit
 
 DESCRIPTION
 -----------
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 0c82ca5bc0..ffce98099c 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -9,9 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[<keyid>]] 
<commit>...
-'git revert' --continue
-'git revert' --quit
-'git revert' --abort
+'git revert' --continue | --skip | --abort | --quit
 
 DESCRIPTION
 -----------
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
index 5a57c4a407..3bceb56474 100644
--- a/Documentation/sequencer.txt
+++ b/Documentation/sequencer.txt
@@ -3,6 +3,10 @@
        `.git/sequencer`.  Can be used to continue after resolving
        conflicts in a failed cherry-pick or revert.
 
+--skip::
+       Skip the current commit and continue with the rest of the
+       sequence.
+
 --quit::
        Forget about the current operation in progress.  Can be used
        to clear the sequencer state after a failed cherry-pick or
diff --git a/builtin/revert.c b/builtin/revert.c
index d4dcedbdc6..5dc5891ea2 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -102,6 +102,7 @@ static int run_sequencer(int argc, const char **argv, 
struct replay_opts *opts)
                OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick 
sequence"), 'q'),
                OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or 
cherry-pick sequence"), 'c'),
                OPT_CMDMODE(0, "abort", &cmd, N_("cancel revert or cherry-pick 
sequence"), 'a'),
+               OPT_CMDMODE(0, "skip", &cmd, N_("skip current commit and 
continue"), 's'),
                OPT_CLEANUP(&cleanup_arg),
                OPT_BOOL('n', "no-commit", &opts->no_commit, N_("don't 
automatically commit")),
                OPT_BOOL('e', "edit", &opts->edit, N_("edit the commit 
message")),
@@ -151,6 +152,8 @@ static int run_sequencer(int argc, const char **argv, 
struct replay_opts *opts)
                        this_operation = "--quit";
                else if (cmd == 'c')
                        this_operation = "--continue";
+               else if (cmd == 's')
+                       this_operation = "--skip";
                else {
                        assert(cmd == 'a');
                        this_operation = "--abort";
@@ -210,6 +213,8 @@ static int run_sequencer(int argc, const char **argv, 
struct replay_opts *opts)
                return sequencer_continue(the_repository, opts);
        if (cmd == 'a')
                return sequencer_rollback(the_repository, opts);
+       if (cmd == 's')
+               return sequencer_skip(the_repository, opts);
        return sequencer_pick_revisions(the_repository, opts);
 }
 
diff --git a/sequencer.c b/sequencer.c
index 9c561a041b..f586e677d3 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2784,6 +2784,29 @@ int sequencer_rollback(struct repository *r, struct 
replay_opts *opts)
        return -1;
 }
 
+int sequencer_skip(struct repository *r, struct replay_opts *opts)
+{
+       switch (opts->action) {
+       case REPLAY_REVERT:
+               if (!file_exists(git_path_revert_head(r)))
+                       return error(_("no revert in progress"));
+               break;
+       case REPLAY_PICK:
+               if (!file_exists(git_path_cherry_pick_head(r)))
+                       return error(_("no cherry-pick in progress"));
+               break;
+       default:
+               BUG("the control must not reach here.");
+       }
+
+       if (rollback_single_pick(r))
+               return error(_("failed to skip the commit"));
+       if (!is_directory(git_path_seq_dir()))
+               return 0;
+
+       return sequencer_continue(r, opts);
+}
+
 static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
 {
        struct lock_file todo_lock = LOCK_INIT;
diff --git a/sequencer.h b/sequencer.h
index 0c494b83d4..731b9853eb 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -129,6 +129,7 @@ int sequencer_pick_revisions(struct repository *repo,
                             struct replay_opts *opts);
 int sequencer_continue(struct repository *repo, struct replay_opts *opts);
 int sequencer_rollback(struct repository *repo, struct replay_opts *opts);
+int sequencer_skip(struct repository *repo, struct replay_opts *opts);
 int sequencer_remove_state(struct replay_opts *opts);
 
 #define TODO_LIST_KEEP_EMPTY (1U << 0)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 941d5026da..48cc9f13ee 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -93,6 +93,69 @@ test_expect_success 'cherry-pick cleans up sequencer state 
upon success' '
        test_path_is_missing .git/sequencer
 '
 
+test_expect_success 'cherry-pick --skip requires cherry-pick in progress' '
+       pristine_detach initial &&
+       test_must_fail git cherry-pick --skip
+'
+
+test_expect_success 'revert --skip requires revert in progress' '
+       pristine_detach initial &&
+       test_must_fail git revert --skip
+'
+
+test_expect_success 'cherry-pick --skip to skip commit' '
+       pristine_detach initial &&
+       test_must_fail git cherry-pick anotherpick &&
+       test_must_fail git revert --skip &&
+       git cherry-pick --skip &&
+       test_cmp_rev initial HEAD &&
+       test_path_is_missing .git/CHERRY_PICK_HEAD
+'
+
+test_expect_success 'revert --skip to skip commit' '
+       pristine_detach anotherpick &&
+       test_must_fail git revert anotherpick~1 &&
+       test_must_fail git cherry-pick --skip &&
+       git revert --skip &&
+       test_cmp_rev anotherpick HEAD
+'
+
+test_expect_success 'skip "empty" commit' '
+       pristine_detach picked &&
+       test_commit dummy foo d &&
+       test_must_fail git cherry-pick anotherpick &&
+       git cherry-pick --skip &&
+       test_cmp_rev dummy HEAD
+'
+
+test_expect_success 'skip a commit and check if rest of sequence is correct' '
+       pristine_detach initial &&
+       echo e >expect &&
+       cat >expect.log <<-EOF &&
+       OBJID
+       :100644 100644 OBJID OBJID M    foo
+       OBJID
+       :100644 100644 OBJID OBJID M    foo
+       OBJID
+       :100644 100644 OBJID OBJID M    unrelated
+       OBJID
+       :000000 100644 OBJID OBJID A    foo
+       :000000 100644 OBJID OBJID A    unrelated
+       EOF
+       test_must_fail git cherry-pick base..yetanotherpick &&
+       test_must_fail git cherry-pick --skip &&
+       echo d >foo &&
+       git add foo &&
+       git cherry-pick --continue &&
+       {
+               git rev-list HEAD |
+               git diff-tree --root --stdin |
+               sed "s/$OID_REGEX/OBJID/g"
+       } >actual.log &&
+       test_cmp expect foo &&
+       test_cmp expect.log actual.log
+'
+
 test_expect_success '--quit does not complain when no cherry-pick is in 
progress' '
        pristine_detach initial &&
        git cherry-pick --quit
-- 
2.21.0

Reply via email to