{"thread":{"id":"51269","subject":"[GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commits","startedAt":"2019-06-08T19:22:57Z","lastAt":"2019-07-02T15:26:34Z","messageCount":95,"participants":["Rohit Ashiwal","Thomas Gummerer","Phillip Wood","Junio C Hamano","Martin Ågren"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"376863","messageId":"20190608191958.4593-1-rohit.ashiwal265@gmail.com","threadId":"51269","inReplyTo":null,"subject":"[GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commits","fromName":"Rohit Ashiwal","fromEmail":"rohit.ashiwal265@gmail.com","sentAt":"2019-06-08T19:19:55Z","receivedAt":"2019-06-08T19:22:57Z","isPatch":true,"sender":{"key":"rohit.ashiwal265@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31043830?v=4"},"body":"git am or rebase advice user to use git am --skip or git rebase --skip\nto skip the commit that has become empty or has risen conflicts. OTOH,\ncherry-pick advice user to use git reset HEAD which on the user’s part\nis annoying and sometimes confusing. This patch series will bring\nconsistency between advices of these commands with introduction of\n`--skip` flag to cherry-pick and revert.\n\nRohit Ashiwal (3):\n sequencer: add advice for revert\n cherry-pick/revert: add --skip option\n cherry-pick/revert: update hints\n\n Documentation/git-cherry-pick.txt | 4 +-\n Documentation/git-revert.txt | 4 +-\n Documentation/sequencer.txt | 4 ++\n builtin/commit.c | 13 ++++---\n builtin/revert.c | 5 +++\n sequencer.c | 26 ++++++++++++-\n sequencer.h | 1 +\n t/t3510-cherry-pick-sequence.sh | 63 +++++++++++++++++++++++++++++++\n 8 files changed, 108 insertions(+), 12 deletions(-)\n\nPR: https://github.com/r1walz/git/pull/1\nReviewed-by: Elijah Newren \nReviewed-by: Thomas Gummerer \n-- \n2.21.0\n\n"},{"id":"376864","messageId":"20190608191958.4593-2-rohit.ashiwal265@gmail.com","threadId":"51269","inReplyTo":"20190608191958.4593-1-rohit.ashiwal265@gmail.com","subject":"[GSoC][PATCH 1/3] sequencer: add advice for revert","fromName":"Rohit Ashiwal","fromEmail":"rohit.ashiwal265@gmail.com","sentAt":"2019-06-08T19:19:56Z","receivedAt":"2019-06-08T19:23:01Z","isPatch":true,"sender":{"key":"rohit.ashiwal265@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31043830?v=4"},"body":"In the case of merge conflicts, while performing a revert, we are\ncurrently advised to use `git cherry-pick --`\nof which --continue is incompatible for continuing the revert.\nIntroduce a separate advice message for `git revert`.\n\nSigned-off-by: Rohit Ashiwal \n---\n sequencer.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex f88a97fb10..9c561a041b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2655,6 +2655,7 @@ static int create_seq_dir(void)\n \tif (file_exists(git_path_seq_dir())) {\n \t\terror(_(\"a cherry-pick or revert is already in progress\"));\n \t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit | --abort)\\\"\"));\n+\t\tadvise(_(\"or \\\"git revert (--continue | --quit | --abort)\\\"\"));\n \t\treturn -1;\n \t} else if (mkdir(git_path_seq_dir(), 0777) < 0)\n \t\treturn error_errno(_(\"could not create sequencer directory '%s'\"),\n-- \n2.21.0\n\n"},{"id":"376865","messageId":"20190608191958.4593-3-rohit.ashiwal265@gmail.com","threadId":"51269","inReplyTo":"20190608191958.4593-1-rohit.ashiwal265@gmail.com","subject":"[GSoC][PATCH 2/3] cherry-pick/revert: add --skip option","fromName":"Rohit Ashiwal","fromEmail":"rohit.ashiwal265@gmail.com","sentAt":"2019-06-08T19:19:57Z","receivedAt":"2019-06-08T19:23:06Z","isPatch":true,"sender":{"key":"rohit.ashiwal265@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31043830?v=4"},"body":"git am or rebase advise the user to use `git (am | rebase) --skip` to\nskip the commit. cherry-pick and revert also have this concept of\nskipping commits but they advise the user to use `git reset` (or in\ncase of a patch which had conflicts, `git reset --merge`) which on the\nuser's part is annoying and sometimes confusing. Add a `--skip` option\nto make these commands more consistent.\n\nIn the next commit, we will change the advice messages hence finishing\nthe process of teaching revert and cherry-pick \"how to skip commits\".\n\nSigned-off-by: Rohit Ashiwal \n---\n Documentation/git-cherry-pick.txt | 4 +-\n Documentation/git-revert.txt | 4 +-\n Documentation/sequencer.txt | 4 ++\n builtin/revert.c | 5 +++\n sequencer.c | 23 +++++++++++\n sequencer.h | 1 +\n t/t3510-cherry-pick-sequence.sh | 63 +++++++++++++++++++++++++++++++\n 7 files changed, 98 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 754b16ce0c..955880ab88 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -10,9 +10,7 @@ SYNOPSIS\n [verse]\n 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff]\n \t\t [-S[]] ...\n-'git cherry-pick' --continue\n-'git cherry-pick' --quit\n-'git cherry-pick' --abort\n+'git cherry-pick' --continue | --skip | --abort | --quit\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex 0c82ca5bc0..ffce98099c 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -9,9 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[]] ...\n-'git revert' --continue\n-'git revert' --quit\n-'git revert' --abort\n+'git revert' --continue | --skip | --abort | --quit\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt\nindex 5a57c4a407..3bceb56474 100644\n--- a/Documentation/sequencer.txt\n+++ b/Documentation/sequencer.txt\n@@ -3,6 +3,10 @@\n \t`.git/sequencer`. Can be used to continue after resolving\n \tconflicts in a failed cherry-pick or revert.\n \n+--skip::\n+\tSkip the current commit and continue with the rest of the\n+\tsequence.\n+\n --quit::\n \tForget about the current operation in progress. Can be used\n \tto clear the sequencer state after a failed cherry-pick or\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex d4dcedbdc6..5dc5891ea2 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -102,6 +102,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n+\t\tOPT_CMDMODE(0, \"skip\", &cmd, N_(\"skip current commit and continue\"), 's'),\n \t\tOPT_CLEANUP(&cleanup_arg),\n \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n@@ -151,6 +152,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \t\t\tthis_operation = \"--quit\";\n \t\telse if (cmd == 'c')\n \t\t\tthis_operation = \"--continue\";\n+\t\telse if (cmd == 's')\n+\t\t\tthis_operation = \"--skip\";\n \t\telse {\n \t\t\tassert(cmd == 'a');\n \t\t\tthis_operation = \"--abort\";\n@@ -210,6 +213,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \t\treturn sequencer_continue(the_repository, opts);\n \tif (cmd == 'a')\n \t\treturn sequencer_rollback(the_repository, opts);\n+\tif (cmd == 's')\n+\t\treturn sequencer_skip(the_repository, opts);\n \treturn sequencer_pick_revisions(the_repository, opts);\n }\n \ndiff --git a/sequencer.c b/sequencer.c\nindex 9c561a041b..f586e677d3 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2784,6 +2784,29 @@ int sequencer_rollback(struct repository *r, struct replay_opts *opts)\n \treturn -1;\n }\n \n+int sequencer_skip(struct repository *r, struct replay_opts *opts)\n+{\n+\tswitch (opts->action) {\n+\tcase REPLAY_REVERT:\n+\t\tif (!file_exists(git_path_revert_head(r)))\n+\t\t\treturn error(_(\"no revert in progress\"));\n+\t\tbreak;\n+\tcase REPLAY_PICK:\n+\t\tif (!file_exists(git_path_cherry_pick_head(r)))\n+\t\t\treturn error(_(\"no cherry-pick in progress\"));\n+\t\tbreak;\n+\tdefault:\n+\t\tBUG(\"the control must not reach here.\");\n+\t}\n+\n+\tif (rollback_single_pick(r))\n+\t\treturn error(_(\"failed to skip the commit\"));\n+\tif (!is_directory(git_path_seq_dir()))\n+\t\treturn 0;\n+\n+\treturn sequencer_continue(r, opts);\n+}\n+\n static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)\n {\n \tstruct lock_file todo_lock = LOCK_INIT;\ndiff --git a/sequencer.h b/sequencer.h\nindex 0c494b83d4..731b9853eb 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -129,6 +129,7 @@ int sequencer_pick_revisions(struct repository *repo,\n \t\t\t struct replay_opts *opts);\n int sequencer_continue(struct repository *repo, struct replay_opts *opts);\n int sequencer_rollback(struct repository *repo, struct replay_opts *opts);\n+int sequencer_skip(struct repository *repo, struct replay_opts *opts);\n int sequencer_remove_state(struct replay_opts *opts);\n \n #define TODO_LIST_KEEP_EMPTY (1U << 0)\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 941d5026da..48cc9f13ee 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -93,6 +93,69 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '\n \ttest_path_is_missing .git/sequencer\n '\n \n+test_expect_success 'cherry-pick --skip requires cherry-pick in progress' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick --skip\n+'\n+\n+test_expect_success 'revert --skip requires revert in progress' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git revert --skip\n+'\n+\n+test_expect_success 'cherry-pick --skip to skip commit' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick anotherpick &&\n+\ttest_must_fail git revert --skip &&\n+\tgit cherry-pick --skip &&\n+\ttest_cmp_rev initial HEAD &&\n+\ttest_path_is_missing .git/CHERRY_PICK_HEAD\n+'\n+\n+test_expect_success 'revert --skip to skip commit' '\n+\tpristine_detach anotherpick &&\n+\ttest_must_fail git revert anotherpick~1 &&\n+\ttest_must_fail git cherry-pick --skip &&\n+\tgit revert --skip &&\n+\ttest_cmp_rev anotherpick HEAD\n+'\n+\n+test_expect_success 'skip \"empty\" commit' '\n+\tpristine_detach picked &&\n+\ttest_commit dummy foo d &&\n+\ttest_must_fail git cherry-pick anotherpick &&\n+\tgit cherry-pick --skip &&\n+\ttest_cmp_rev dummy HEAD\n+'\n+\n+test_expect_success 'skip a commit and check if rest of sequence is correct' '\n+\tpristine_detach initial &&\n+\techo e >expect &&\n+\tcat >expect.log <<-EOF &&\n+\tOBJID\n+\t:100644 100644 OBJID OBJID M\tfoo\n+\tOBJID\n+\t:100644 100644 OBJID OBJID M\tfoo\n+\tOBJID\n+\t:100644 100644 OBJID OBJID M\tunrelated\n+\tOBJID\n+\t:000000 100644 OBJID OBJID A\tfoo\n+\t:000000 100644 OBJID OBJID A\tunrelated\n+\tEOF\n+\ttest_must_fail git cherry-pick base..yetanotherpick &&\n+\ttest_must_fail git cherry-pick --skip &&\n+\techo d >foo &&\n+\tgit add foo &&\n+\tgit cherry-pick --continue &&\n+\t{\n+\t\tgit rev-list HEAD |\n+\t\tgit diff-tree --root --stdin |\n+\t\tsed \"s/$OID_REGEX/OBJID/g\"\n+\t} >actual.log &&\n+\ttest_cmp expect foo &&\n+\ttest_cmp expect.log actual.log\n+'\n+\n test_expect_success '--quit does not complain when no cherry-pick is in progress' '\n \tpristine_detach initial &&\n \tgit cherry-pick --quit\n-- \n2.21.0\n\n"},{"id":"376866","messageId":"20190608191958.4593-4-rohit.ashiwal265@gmail.com","threadId":"51269","inReplyTo":"20190608191958.4593-1-rohit.ashiwal265@gmail.com","subject":"[GSoC][PATCH 3/3] cherry-pick/revert: update hints","fromName":"Rohit Ashiwal","fromEmail":"rohit.ashiwal265@gmail.com","sentAt":"2019-06-08T19:19:58Z","receivedAt":"2019-06-08T19:23:09Z","isPatch":true,"sender":{"key":"rohit.ashiwal265@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31043830?v=4"},"body":"Signed-off-by: Rohit Ashiwal \n---\n builtin/commit.c | 13 ++++++++-----\n sequencer.c | 4 ++--\n 2 files changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 1c9e8e2228..1f47c51bdc 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -60,15 +60,18 @@ N_(\"The previous cherry-pick is now empty, possibly due to conflict resolution.\\\n \"\\n\");\n \n static const char empty_cherry_pick_advice_single[] =\n-N_(\"Otherwise, please use 'git reset'\\n\");\n+N_(\"Otherwise, please use 'git cherry-pick --skip'\\n\");\n \n static const char empty_cherry_pick_advice_multi[] =\n-N_(\"If you wish to skip this commit, use:\\n\"\n+N_(\"and then use:\\n\"\n \"\\n\"\n-\" git reset\\n\"\n+\" git cherry-pick --continue\\n\"\n \"\\n\"\n-\"Then \\\"git cherry-pick --continue\\\" will resume cherry-picking\\n\"\n-\"the remaining commits.\\n\");\n+\"to resume cherry-picking the remaining commits.\\n\"\n+\"If you wish to skip this commit, use:\\n\"\n+\"\\n\"\n+\" git cherry-pick --skip\\n\"\n+\"\\n\");\n \n static const char *color_status_slots[] = {\n \t[WT_STATUS_HEADER]\t = \"header\",\ndiff --git a/sequencer.c b/sequencer.c\nindex f586e677d3..e889427eef 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2654,8 +2654,8 @@ static int create_seq_dir(void)\n {\n \tif (file_exists(git_path_seq_dir())) {\n \t\terror(_(\"a cherry-pick or revert is already in progress\"));\n-\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit | --abort)\\\"\"));\n-\t\tadvise(_(\"or \\\"git revert (--continue | --quit | --abort)\\\"\"));\n+\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --skip | --quit | --abort)\\\"\"));\n+\t\tadvise(_(\"or \\\"git revert (--continue | --skip | --quit | --abort)\\\"\"));\n \t\treturn -1;\n \t} else if (mkdir(git_path_seq_dir(), 0777) < 0)\n \t\treturn error_errno(_(\"could not create sequencer directory '%s'\"),\n-- \n2.21.0\n\n"},{"id":"376876","messageId":"20190609083708.GA28007@hank.intra.tgummerer.com","threadId":"51269","inReplyTo":"20190608191958.4593-3-rohit.ashiwal265@gmail.com","subject":"Re: [GSoC][PATCH 2/3] cherry-pick/revert: add --skip option","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-06-09T08:37:09Z","receivedAt":"2019-06-09T08:37:17Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 06/09, Rohit Ashiwal wrote:\n> git am or rebase advise the user to use `git (am | rebase) --skip` to\n> skip the commit. cherry-pick and revert also have this concept of\n> skipping commits but they advise the user to use `git reset` (or in\n> case of a patch which had conflicts, `git reset --merge`) which on the\n> user's part is annoying and sometimes confusing. Add a `--skip` option\n> to make these commands more consistent.\n\nSomething I missed in the off-list review for this PR: I think this\ncommit message is a bit heavy on the advice part, rather than on the\nreal advantage the --skip flag brings, which is that it's less\ncumbersome for the user to skip a commit, and brings consistency to\nthe commands. Maybe something like:\n\n git am or rebase have a --skip flag to skip the current commit if\n the user wishes to do so. During a cherry-pick or revert a user\n could likewise skip a commit, but needs to use 'git reset' (or in\n the case of conflicts 'git reset --merge'), followed by 'git\n (cherry-pick | revert) --continue' to skip the commit. This is\n more annoying and sometimes confusing on the users part. Add a\n `--skip` option to make skipping commits easier for the user and\n to make the commands more consistent.\n\n> In the next commit, we will change the advice messages hence finishing\n> the process of teaching revert and cherry-pick \"how to skip commits\".\n> \n> Signed-off-by: Rohit Ashiwal \n> ---\n> Documentation/git-cherry-pick.txt | 4 +-\n> Documentation/git-revert.txt | 4 +-\n> Documentation/sequencer.txt | 4 ++\n> builtin/revert.c | 5 +++\n> sequencer.c | 23 +++++++++++\n> sequencer.h | 1 +\n> t/t3510-cherry-pick-sequence.sh | 63 +++++++++++++++++++++++++++++++\n> 7 files changed, 98 insertions(+), 6 deletions(-)\n> \n> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\n> index 754b16ce0c..955880ab88 100644\n> --- a/Documentation/git-cherry-pick.txt\n> +++ b/Documentation/git-cherry-pick.txt\n> @@ -10,9 +10,7 @@ SYNOPSIS\n> [verse]\n> 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff]\n> \t\t [-S[]] ...\n> -'git cherry-pick' --continue\n> -'git cherry-pick' --quit\n> -'git cherry-pick' --abort\n> +'git cherry-pick' --continue | --skip | --abort | --quit\n> \n> DESCRIPTION\n> -----------\n> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> index 0c82ca5bc0..ffce98099c 100644\n> --- a/Documentation/git-revert.txt\n> +++ b/Documentation/git-revert.txt\n> @@ -9,9 +9,7 @@ SYNOPSIS\n> --------\n> [verse]\n> 'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[]] ...\n> -'git revert' --continue\n> -'git revert' --quit\n> -'git revert' --abort\n> +'git revert' --continue | --skip | --abort | --quit\n> \n> DESCRIPTION\n> -----------\n> diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt\n> index 5a57c4a407..3bceb56474 100644\n> --- a/Documentation/sequencer.txt\n> +++ b/Documentation/sequencer.txt\n> @@ -3,6 +3,10 @@\n> \t`.git/sequencer`. Can be used to continue after resolving\n> \tconflicts in a failed cherry-pick or revert.\n> \n> +--skip::\n> +\tSkip the current commit and continue with the rest of the\n> +\tsequence.\n> +\n> --quit::\n> \tForget about the current operation in progress. Can be used\n> \tto clear the sequencer state after a failed cherry-pick or\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index d4dcedbdc6..5dc5891ea2 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -102,6 +102,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n> \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n> \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n> +\t\tOPT_CMDMODE(0, \"skip\", &cmd, N_(\"skip current commit and continue\"), 's'),\n> \t\tOPT_CLEANUP(&cleanup_arg),\n> \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n> \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n> @@ -151,6 +152,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> \t\t\tthis_operation = \"--quit\";\n> \t\telse if (cmd == 'c')\n> \t\t\tthis_operation = \"--continue\";\n> +\t\telse if (cmd == 's')\n> +\t\t\tthis_operation = \"--skip\";\n> \t\telse {\n> \t\t\tassert(cmd == 'a');\n> \t\t\tthis_operation = \"--abort\";\n> @@ -210,6 +213,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> \t\treturn sequencer_continue(the_repository, opts);\n> \tif (cmd == 'a')\n> \t\treturn sequencer_rollback(the_repository, opts);\n> +\tif (cmd == 's')\n> +\t\treturn sequencer_skip(the_repository, opts);\n> \treturn sequencer_pick_revisions(the_repository, opts);\n> }\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index 9c561a041b..f586e677d3 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2784,6 +2784,29 @@ int sequencer_rollback(struct repository *r, struct replay_opts *opts)\n> \treturn -1;\n> }\n> \n> +int sequencer_skip(struct repository *r, struct replay_opts *opts)\n> +{\n> +\tswitch (opts->action) {\n> +\tcase REPLAY_REVERT:\n> +\t\tif (!file_exists(git_path_revert_head(r)))\n> +\t\t\treturn error(_(\"no revert in progress\"));\n> +\t\tbreak;\n> +\tcase REPLAY_PICK:\n> +\t\tif (!file_exists(git_path_cherry_pick_head(r)))\n> +\t\t\treturn error(_(\"no cherry-pick in progress\"));\n> +\t\tbreak;\n\nI see that in some cases (e.g 'rollback_single_pick'), we just check\nif either REVERT_HEAD or CHERRY_PICK_HEAD exist, but would allow the\n\"wrong\" command to be used, e.g. 'git revert --abort' when a\ncherry-pick is in progress. This helps avoiding that, good!\n\n> +\tdefault:\n> +\t\tBUG(\"the control must not reach here.\");\n\nWe don't support rebase in 'sequencer_skip' yet, makes sense as this\npatch is focused on cherry-pick and revert. There's nothing\npreventing supporting that in the future though.\n\n> +\t}\n> +\n> +\tif (rollback_single_pick(r))\n> +\t\treturn error(_(\"failed to skip the commit\"));\n> +\tif (!is_directory(git_path_seq_dir()))\n> +\t\treturn 0;\n\nIf there's no sequencer directory, we don't need to continue the\ncherry-pick/revert, as there's nothing left to do. Otherwise we\ncontinue. Good.\n\nSo this looks good to me.\n\n> +\n> +\treturn sequencer_continue(r, opts);\n> +}\n> +\n> static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)\n> {\n> \tstruct lock_file todo_lock = LOCK_INIT;\n> diff --git a/sequencer.h b/sequencer.h\n> index 0c494b83d4..731b9853eb 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -129,6 +129,7 @@ int sequencer_pick_revisions(struct repository *repo,\n> \t\t\t struct replay_opts *opts);\n> int sequencer_continue(struct repository *repo, struct replay_opts *opts);\n> int sequencer_rollback(struct repository *repo, struct replay_opts *opts);\n> +int sequencer_skip(struct repository *repo, struct replay_opts *opts);\n> int sequencer_remove_state(struct replay_opts *opts);\n> \n> #define TODO_LIST_KEEP_EMPTY (1U << 0)\n> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\n> index 941d5026da..48cc9f13ee 100755\n> --- a/t/t3510-cherry-pick-sequence.sh\n> +++ b/t/t3510-cherry-pick-sequence.sh\n> @@ -93,6 +93,69 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '\n> \ttest_path_is_missing .git/sequencer\n> '\n> \n> +test_expect_success 'cherry-pick --skip requires cherry-pick in progress' '\n> +\tpristine_detach initial &&\n> +\ttest_must_fail git cherry-pick --skip\n> +'\n> +\n> +test_expect_success 'revert --skip requires revert in progress' '\n> +\tpristine_detach initial &&\n> +\ttest_must_fail git revert --skip\n> +'\n> +\n> +test_expect_success 'cherry-pick --skip to skip commit' '\n> +\tpristine_detach initial &&\n> +\ttest_must_fail git cherry-pick anotherpick &&\n> +\ttest_must_fail git revert --skip &&\n> +\tgit cherry-pick --skip &&\n> +\ttest_cmp_rev initial HEAD &&\n> +\ttest_path_is_missing .git/CHERRY_PICK_HEAD\n> +'\n> +\n> +test_expect_success 'revert --skip to skip commit' '\n> +\tpristine_detach anotherpick &&\n> +\ttest_must_fail git revert anotherpick~1 &&\n> +\ttest_must_fail git cherry-pick --skip &&\n> +\tgit revert --skip &&\n> +\ttest_cmp_rev anotherpick HEAD\n> +'\n> +\n> +test_expect_success 'skip \"empty\" commit' '\n> +\tpristine_detach picked &&\n> +\ttest_commit dummy foo d &&\n> +\ttest_must_fail git cherry-pick anotherpick &&\n> +\tgit cherry-pick --skip &&\n> +\ttest_cmp_rev dummy HEAD\n> +'\n> +\n> +test_expect_success 'skip a commit and check if rest of sequence is correct' '\n> +\tpristine_detach initial &&\n> +\techo e >expect &&\n> +\tcat >expect.log <<-EOF &&\n> +\tOBJID\n> +\t:100644 100644 OBJID OBJID M\tfoo\n> +\tOBJID\n> +\t:100644 100644 OBJID OBJID M\tfoo\n> +\tOBJID\n> +\t:100644 100644 OBJID OBJID M\tunrelated\n> +\tOBJID\n> +\t:000000 100644 OBJID OBJID A\tfoo\n> +\t:000000 100644 OBJID OBJID A\tunrelated\n> +\tEOF\n> +\ttest_must_fail git cherry-pick base..yetanotherpick &&\n> +\ttest_must_fail git cherry-pick --skip &&\n> +\techo d >foo &&\n> +\tgit add foo &&\n> +\tgit cherry-pick --continue &&\n> +\t{\n> +\t\tgit rev-list HEAD |\n> +\t\tgit diff-tree --root --stdin |\n> +\t\tsed \"s/$OID_REGEX/OBJID/g\"\n> +\t} >actual.log &&\n> +\ttest_cmp expect foo &&\n> +\ttest_cmp expect.log actual.log\n> +'\n> +\n> test_expect_success '--quit does not complain when no cherry-pick is in progress' '\n> \tpristine_detach initial &&\n> \tgit cherry-pick --quit\n> -- \n> 2.21.0\n> \n"},{"id":"376877","messageId":"20190609084236.GB28007@hank.intra.tgummerer.com","threadId":"51269","inReplyTo":"20190608191958.4593-4-rohit.ashiwal265@gmail.com","subject":"Re: [GSoC][PATCH 3/3] cherry-pick/revert: update hints","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-06-09T08:42:36Z","receivedAt":"2019-06-09T08:42:43Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"> Subject: [GSoC][PATCH 3/3] cherry-pick/revert: update hints\n\n\"update hints\" is very generic when read in 'git log --oneline', and\ndoesn't give the reader much useful information. Maybe something like\n\n cherry-pick/revert: advice using --skip\n\n In the previous commit we introduced a --skip flag for\n cherry-pick and revert. Update the advice messages, to tell\n users about this less cumbersome way of skipping commits\n\nwould work better? The rest of the patch looks good to me.\n\nOn 06/09, Rohit Ashiwal wrote:\n> Signed-off-by: Rohit Ashiwal \n> ---\n> builtin/commit.c | 13 ++++++++-----\n> sequencer.c | 4 ++--\n> 2 files changed, 10 insertions(+), 7 deletions(-)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 1c9e8e2228..1f47c51bdc 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -60,15 +60,18 @@ N_(\"The previous cherry-pick is now empty, possibly due to conflict resolution.\\\n> \"\\n\");\n> \n> static const char empty_cherry_pick_advice_single[] =\n> -N_(\"Otherwise, please use 'git reset'\\n\");\n> +N_(\"Otherwise, please use 'git cherry-pick --skip'\\n\");\n> \n> static const char empty_cherry_pick_advice_multi[] =\n> -N_(\"If you wish to skip this commit, use:\\n\"\n> +N_(\"and then use:\\n\"\n> \"\\n\"\n> -\" git reset\\n\"\n> +\" git cherry-pick --continue\\n\"\n> \"\\n\"\n> -\"Then \\\"git cherry-pick --continue\\\" will resume cherry-picking\\n\"\n> -\"the remaining commits.\\n\");\n> +\"to resume cherry-picking the remaining commits.\\n\"\n> +\"If you wish to skip this commit, use:\\n\"\n> +\"\\n\"\n> +\" git cherry-pick --skip\\n\"\n> +\"\\n\");\n> \n> static const char *color_status_slots[] = {\n> \t[WT_STATUS_HEADER]\t = \"header\",\n> diff --git a/sequencer.c b/sequencer.c\n> index f586e677d3..e889427eef 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2654,8 +2654,8 @@ static int create_seq_dir(void)\n> {\n> \tif (file_exists(git_path_seq_dir())) {\n> \t\terror(_(\"a cherry-pick or revert is already in progress\"));\n> -\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit | --abort)\\\"\"));\n> -\t\tadvise(_(\"or \\\"git revert (--continue | --quit | --abort)\\\"\"));\n> +\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --skip | --quit | --abort)\\\"\"));\n> +\t\tadvise(_(\"or \\\"git revert (--continue | --skip | --quit | --abort)\\\"\"));\n> \t\treturn -1;\n> \t} else if (mkdir(git_path_seq_dir(), 0777) < 0)\n> \t\treturn error_errno(_(\"could not create sequencer directory '%s'\"),\n> -- \n> 2.21.0\n> \n"},{"id":"376878","messageId":"20190609090235.GC28007@hank.intra.tgummerer.com","threadId":"51269","inReplyTo":"20190608191958.4593-1-rohit.ashiwal265@gmail.com","subject":"Re: [GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commits","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-06-09T09:02:35Z","receivedAt":"2019-06-09T09:02:41Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 06/09, Rohit Ashiwal wrote:\n> git am or rebase advice user to use git am --skip or git rebase --skip\n> to skip the commit that has become empty or has risen conflicts. OTOH,\n> cherry-pick advice user to use git reset HEAD which on the user’s part\n> is annoying and sometimes confusing. This patch series will bring\n> consistency between advices of these commands with introduction of\n> `--skip` flag to cherry-pick and revert.\n\nThanks! I found a few minor nits that I missed in my off-list review\non the PR. We should give others some time to comment now before you\nre-send it with the nits fixed (if you agree with them).\n\nOne thing that would be good to add to future cover letters is a\nreminder of how this fits in to your GSoC project, where the overall\nend goal is to improve the consistency of sequencer commands, and this\nis one step along that way.\n\n> Rohit Ashiwal (3):\n> sequencer: add advice for revert\n> cherry-pick/revert: add --skip option\n> cherry-pick/revert: update hints\n> \n> Documentation/git-cherry-pick.txt | 4 +-\n> Documentation/git-revert.txt | 4 +-\n> Documentation/sequencer.txt | 4 ++\n> builtin/commit.c | 13 ++++---\n> builtin/revert.c | 5 +++\n> sequencer.c | 26 ++++++++++++-\n> sequencer.h | 1 +\n> t/t3510-cherry-pick-sequence.sh | 63 +++++++++++++++++++++++++++++++\n> 8 files changed, 108 insertions(+), 12 deletions(-)\n> \n> PR: https://github.com/r1walz/git/pull/1\n> Reviewed-by: Elijah Newren \n> Reviewed-by: Thomas Gummerer \n\nNote that the 'Reviewed-by' footer is something that is \"given\" by the\nreviewers, and should only be added after they have explicitly given\nit for the patch/series in question. Instead this should probably\nmention that Elijah and me reviewed this off-list in the PR you linked\nto above.\n\n> -- \n> 2.21.0\n> \n"},{"id":"376880","messageId":"CAL7ArXpMBXwOGXWmYx8Rv9bj2trJ-FS_VGgPmJbAh+evbaxNEw@mail.gmail.com","threadId":"51269","inReplyTo":"20190609090235.GC28007@hank.intra.tgummerer.com","subject":"Re: [GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commits","fromName":"Rohit Ashiwal","fromEmail":"rohit.ashiwal265@gmail.com","sentAt":"2019-06-09T10:06:23Z","receivedAt":"2019-06-09T10:09:21Z","isPatch":true,"sender":{"key":"rohit.ashiwal265@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31043830?v=4"},"body":"Hey Thomas\n\nOn Sun, Jun 9, 2019 at 2:32 PM Thomas Gummerer wrote:\n>\n> Thanks! I found a few minor nits that I missed in my off-list review\n> on the PR. We should give others some time to comment now before you\n> re-send it with the nits fixed (if you agree with them).\n\nYes, I'll wait for some more reviews before sending another iteration.\n\n> One thing that would be good to add to future cover letters is a\n> reminder of how this fits in to your GSoC project, where the overall\n> end goal is to improve the consistency of sequencer commands, and this\n> is one step along that way.\n\nSure, I'll keep this in mind and send some kind of summary in the next\nrevision.\n\n>\n> > PR: https://github.com/r1walz/git/pull/1\n> > Reviewed-by: Elijah Newren \n> > Reviewed-by: Thomas Gummerer \n>\n> Note that the 'Reviewed-by' footer is something that is \"given\" by the\n> reviewers, and should only be added after they have explicitly given\n> it for the patch/series in question. Instead this should probably\n> mention that Elijah and me reviewed this off-list in the PR you linked\n> to above.\n\nOh! But I read here, SubmittingPatches[1] docs, that it's nice to give\ncredits to those who helped you. Should we fix it in some way? I don't\nknow. Anyway, that is beyond the scope of this patch.\n\n>\n> > --\n> > 2.21.0\n> >\n\nThanks for the review\nRohit\n\n[1]: https://github.com/git/git/blob/v2.22.0/Documentation/SubmittingPatches#L289-L291\n"},{"id":"376888","messageId":"2007ef79-1791-4f77-f9e4-34c59b4f0c36@gmail.com","threadId":"51269","inReplyTo":"20190608191958.4593-2-rohit.ashiwal265@gmail.com","subject":"Re: [GSoC][PATCH 1/3] sequencer: add advice for revert","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-06-09T17:52:42Z","receivedAt":"2019-06-09T17:56:01Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Rohit\n\nCongratulations on your first GSoC patch series!\n\nOn 08/06/2019 20:19, Rohit Ashiwal wrote:\n> In the case of merge conflicts, while performing a revert, we are\n> currently advised to use `git cherry-pick --`\n> of which --continue is incompatible for continuing the revert.\n> Introduce a separate advice message for `git revert`.\n> \n> Signed-off-by: Rohit Ashiwal \n> ---\n> sequencer.c | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index f88a97fb10..9c561a041b 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2655,6 +2655,7 @@ static int create_seq_dir(void)\n> \tif (file_exists(git_path_seq_dir())) {\n> \t\terror(_(\"a cherry-pick or revert is already in progress\"));\n> \t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit | --abort)\\\"\"));\n> +\t\tadvise(_(\"or \\\"git revert (--continue | --quit | --abort)\\\"\"));\n\nI agree that it's a good idea to add advice for revert as well, but it\nwould be better to call sequencer_get_last_command() to find out if\nwe're running cherry-pick or revert and tailor the advice appropriately.\n\nBest Wishes\n\nPhillip\n\n> \t\treturn -1;\n> \t} else if (mkdir(git_path_seq_dir(), 0777) < 0)\n> \t\treturn error_errno(_(\"could not create sequencer directory '%s'\"),\n> \n\n"},{"id":"376889","messageId":"0659a53e-0712-eb50-f15f-1c6a56fd3213@gmail.com","threadId":"51269","inReplyTo":"20190608191958.4593-3-rohit.ashiwal265@gmail.com","subject":"Re: [GSoC][PATCH 2/3] cherry-pick/revert: add --skip option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-06-09T18:01:35Z","receivedAt":"2019-06-09T18:06:44Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Rohit\n\n--skip is definitely a useful addition to cherry-pick/revert\n\nOn 08/06/2019 20:19, Rohit Ashiwal wrote:\n> git am or rebase advise the user to use `git (am | rebase) --skip` to\n> skip the commit. cherry-pick and revert also have this concept of\n> skipping commits but they advise the user to use `git reset` (or in\n> case of a patch which had conflicts, `git reset --merge`) which on the\n> user's part is annoying and sometimes confusing. Add a `--skip` option\n> to make these commands more consistent.\n> \n> In the next commit, we will change the advice messages hence finishing\n> the process of teaching revert and cherry-pick \"how to skip commits\".\n> \n> Signed-off-by: Rohit Ashiwal \n> ---\n> Documentation/git-cherry-pick.txt | 4 +-\n> Documentation/git-revert.txt | 4 +-\n> Documentation/sequencer.txt | 4 ++\n> builtin/revert.c | 5 +++\n> sequencer.c | 23 +++++++++++\n> sequencer.h | 1 +\n> t/t3510-cherry-pick-sequence.sh | 63 +++++++++++++++++++++++++++++++\n> 7 files changed, 98 insertions(+), 6 deletions(-)\n> \n> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\n> index 754b16ce0c..955880ab88 100644\n> --- a/Documentation/git-cherry-pick.txt\n> +++ b/Documentation/git-cherry-pick.txt\n> @@ -10,9 +10,7 @@ SYNOPSIS\n> [verse]\n> 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff]\n> \t\t [-S[]] ...\n> -'git cherry-pick' --continue\n> -'git cherry-pick' --quit\n> -'git cherry-pick' --abort\n> +'git cherry-pick' --continue | --skip | --abort | --quit\n> \n> DESCRIPTION\n> -----------\n> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\n> index 0c82ca5bc0..ffce98099c 100644\n> --- a/Documentation/git-revert.txt\n> +++ b/Documentation/git-revert.txt\n> @@ -9,9 +9,7 @@ SYNOPSIS\n> --------\n> [verse]\n> 'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[]] ...\n> -'git revert' --continue\n> -'git revert' --quit\n> -'git revert' --abort\n> +'git revert' --continue | --skip | --abort | --quit\n> \n> DESCRIPTION\n> -----------\n> diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt\n> index 5a57c4a407..3bceb56474 100644\n> --- a/Documentation/sequencer.txt\n> +++ b/Documentation/sequencer.txt\n> @@ -3,6 +3,10 @@\n> \t`.git/sequencer`. Can be used to continue after resolving\n> \tconflicts in a failed cherry-pick or revert.\n> \n> +--skip::\n> +\tSkip the current commit and continue with the rest of the\n> +\tsequence.\n> +\n> --quit::\n> \tForget about the current operation in progress. Can be used\n> \tto clear the sequencer state after a failed cherry-pick or\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index d4dcedbdc6..5dc5891ea2 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -102,6 +102,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> \t\tOPT_CMDMODE(0, \"quit\", &cmd, N_(\"end revert or cherry-pick sequence\"), 'q'),\n> \t\tOPT_CMDMODE(0, \"continue\", &cmd, N_(\"resume revert or cherry-pick sequence\"), 'c'),\n> \t\tOPT_CMDMODE(0, \"abort\", &cmd, N_(\"cancel revert or cherry-pick sequence\"), 'a'),\n> +\t\tOPT_CMDMODE(0, \"skip\", &cmd, N_(\"skip current commit and continue\"), 's'),\n> \t\tOPT_CLEANUP(&cleanup_arg),\n> \t\tOPT_BOOL('n', \"no-commit\", &opts->no_commit, N_(\"don't automatically commit\")),\n> \t\tOPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n> @@ -151,6 +152,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> \t\t\tthis_operation = \"--quit\";\n> \t\telse if (cmd == 'c')\n> \t\t\tthis_operation = \"--continue\";\n> +\t\telse if (cmd == 's')\n> +\t\t\tthis_operation = \"--skip\";\n> \t\telse {\n> \t\t\tassert(cmd == 'a');\n> \t\t\tthis_operation = \"--abort\";\n> @@ -210,6 +213,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n> \t\treturn sequencer_continue(the_repository, opts);\n> \tif (cmd == 'a')\n> \t\treturn sequencer_rollback(the_repository, opts);\n> +\tif (cmd == 's')\n> +\t\treturn sequencer_skip(the_repository, opts);\n> \treturn sequencer_pick_revisions(the_repository, opts);\n> }\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index 9c561a041b..f586e677d3 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2784,6 +2784,29 @@ int sequencer_rollback(struct repository *r, struct replay_opts *opts)\n> \treturn -1;\n> }\n> \n> +int sequencer_skip(struct repository *r, struct replay_opts *opts)\n> +{\n> +\tswitch (opts->action) {\n> +\tcase REPLAY_REVERT:\n> +\t\tif (!file_exists(git_path_revert_head(r)))\n> +\t\t\treturn error(_(\"no revert in progress\"));\n\nThis error message is slightly misleading. If the user has already\ncommitted a conflict resolution then REVERT_HEAD/CHERRY_PICK_HEAD will\nnot exist but it is possible that a cherry-pick/revert is in progress if\nthe user was picking a sequence of commits. It would be nice to give a\ndifferent error message or maybe just a warning in that case.\nsequencer_get_last_command() should help with that.\n\n> +\t\tbreak;\n> +\tcase REPLAY_PICK:\n> +\t\tif (!file_exists(git_path_cherry_pick_head(r)))\n> +\t\t\treturn error(_(\"no cherry-pick in progress\"));\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tBUG(\"the control must not reach here.\");\n> +\t}\n> +\n> +\tif (rollback_single_pick(r))\n> +\t\treturn error(_(\"failed to skip the commit\"));\n\nIf rollback_single_pick() sees that HEAD is the null oid then it gives\nthe error \"cannot abort from a branch yet to be born\". We're not\naborting and if we're picking a sequence of commits the skip ought\nsucceed, but it won't at the moment. If we're picking a single commit\nthen the skip should probably fail like abort but with an appropriate\nmessage. Admittedly that's all a bit of a corner case.\n\n> +\tif (!is_directory(git_path_seq_dir()))\n> +\t\treturn 0;\n> +\n> +\treturn sequencer_continue(r, opts);\n> +}\n> +\n> static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)\n> {\n> \tstruct lock_file todo_lock = LOCK_INIT;\n> diff --git a/sequencer.h b/sequencer.h\n> index 0c494b83d4..731b9853eb 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -129,6 +129,7 @@ int sequencer_pick_revisions(struct repository *repo,\n> \t\t\t struct replay_opts *opts);\n> int sequencer_continue(struct repository *repo, struct replay_opts *opts);\n> int sequencer_rollback(struct repository *repo, struct replay_opts *opts);\n> +int sequencer_skip(struct repository *repo, struct replay_opts *opts);\n> int sequencer_remove_state(struct replay_opts *opts);\n> \n> #define TODO_LIST_KEEP_EMPTY (1U << 0)\n> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\n> index 941d5026da..48cc9f13ee 100755\n> --- a/t/t3510-cherry-pick-sequence.sh\n> +++ b/t/t3510-cherry-pick-sequence.sh\n> @@ -93,6 +93,69 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '\n> \ttest_path_is_missing .git/sequencer\n> '\n> \n> +test_expect_success 'cherry-pick --skip requires cherry-pick in progress' '\n> +\tpristine_detach initial &&\n> +\ttest_must_fail git cherry-pick --skip\n> +'\n> +\n> +test_expect_success 'revert --skip requires revert in progress' '\n> +\tpristine_detach initial &&\n> +\ttest_must_fail git revert --skip\n> +'\n> +\n> +test_expect_success 'cherry-pick --skip to skip commit' '\n> +\tpristine_detach initial &&\n> +\ttest_must_fail git cherry-pick anotherpick &&\n> +\ttest_must_fail git revert --skip &&\n> +\tgit cherry-pick --skip &&\n> +\ttest_cmp_rev initial HEAD &&\n> +\ttest_path_is_missing .git/CHERRY_PICK_HEAD\n> +'\n> +\n> +test_expect_success 'revert --skip to skip commit' '\n> +\tpristine_detach anotherpick &&\n> +\ttest_must_fail git revert anotherpick~1 &&\n> +\ttest_must_fail git cherry-pick --skip &&\n> +\tgit revert --skip &&\n> +\ttest_cmp_rev anotherpick HEAD\n> +'\n> +\n> +test_expect_success 'skip \"empty\" commit' '\n> +\tpristine_detach picked &&\n> +\ttest_commit dummy foo d &&\n> +\ttest_must_fail git cherry-pick anotherpick &&\n> +\tgit cherry-pick --skip &&\n> +\ttest_cmp_rev dummy HEAD\n> +'\n> +\n> +test_expect_success 'skip a commit and check if rest of sequence is correct' '\n> +\tpristine_detach initial &&\n> +\techo e >expect &&\n> +\tcat >expect.log <<-EOF &&\n> +\tOBJID\n> +\t:100644 100644 OBJID OBJID M\tfoo\n> +\tOBJID\n> +\t:100644 100644 OBJID OBJID M\tfoo\n> +\tOBJID\n> +\t:100644 100644 OBJID OBJID M\tunrelated\n> +\tOBJID\n> +\t:000000 100644 OBJID OBJID A\tfoo\n> +\t:000000 100644 OBJID OBJID A\tunrelated\n> +\tEOF\n> +\ttest_must_fail git cherry-pick base..yetanotherpick &&\n> +\ttest_must_fail git cherry-pick --skip &&\n\nIt would be good to check that the cherry-pick has progressed since\n--skip and didn't just fail without trying to pick another commit.\n\n\nBest Wishes\n\nPhillip\n\n> +\techo d >foo &&\n> +\tgit add foo &&\n> +\tgit cherry-pick --continue &&\n> +\t{\n> +\t\tgit rev-list HEAD |\n> +\t\tgit diff-tree --root --stdin |\n> +\t\tsed \"s/$OID_REGEX/OBJID/g\"\n> +\t} >actual.log &&\n> +\ttest_cmp expect foo &&\n> +\ttest_cmp expect.log actual.log\n> +'\n> +\n> test_expect_success '--quit does not complain when no cherry-pick is in progress' '\n> \tpristine_detach initial &&\n> \tgit cherry-pick --quit\n> \n\n"},{"id":"376890","messageId":"61a8bfde-018c-5a69-5abd-89d4a332e39e@gmail.com","threadId":"51269","inReplyTo":"20190608191958.4593-4-rohit.ashiwal265@gmail.com","subject":"Re: [GSoC][PATCH 3/3] cherry-pick/revert: update hints","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-06-09T18:03:02Z","receivedAt":"2019-06-09T18:06:44Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Rohit\n\nOn 08/06/2019 20:19, Rohit Ashiwal wrote:\n> Signed-off-by: Rohit Ashiwal \n> ---\n> builtin/commit.c | 13 ++++++++-----\n> sequencer.c | 4 ++--\n> 2 files changed, 10 insertions(+), 7 deletions(-)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 1c9e8e2228..1f47c51bdc 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -60,15 +60,18 @@ N_(\"The previous cherry-pick is now empty, possibly due to conflict resolution.\\\n> \"\\n\");\n> \n> static const char empty_cherry_pick_advice_single[] =\n> -N_(\"Otherwise, please use 'git reset'\\n\");\n> +N_(\"Otherwise, please use 'git cherry-pick --skip'\\n\");\n> \n> static const char empty_cherry_pick_advice_multi[] =\n> -N_(\"If you wish to skip this commit, use:\\n\"\n> +N_(\"and then use:\\n\"\n> \"\\n\"\n> -\" git reset\\n\"\n> +\" git cherry-pick --continue\\n\"\n> \"\\n\"\n> -\"Then \\\"git cherry-pick --continue\\\" will resume cherry-picking\\n\"\n> -\"the remaining commits.\\n\");\n> +\"to resume cherry-picking the remaining commits.\\n\"\n> +\"If you wish to skip this commit, use:\\n\"\n> +\"\\n\"\n> +\" git cherry-pick --skip\\n\"\n> +\"\\n\");\n> \n> static const char *color_status_slots[] = {\n> \t[WT_STATUS_HEADER]\t = \"header\",\n> diff --git a/sequencer.c b/sequencer.c\n> index f586e677d3..e889427eef 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2654,8 +2654,8 @@ static int create_seq_dir(void)\n> {\n> \tif (file_exists(git_path_seq_dir())) {\n> \t\terror(_(\"a cherry-pick or revert is already in progress\"));\n> -\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit | --abort)\\\"\"));\n> -\t\tadvise(_(\"or \\\"git revert (--continue | --quit | --abort)\\\"\"));\n> +\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --skip | --quit | --abort)\\\"\"));\n> +\t\tadvise(_(\"or \\\"git revert (--continue | --skip | --quit | --abort)\\\"\"));\n\nIf the user has already committed the conflict resolution then we don't\nwant to recommend --skip as there is nothing to skip.\n\nBest Wishes\n\nPhillip\n\n> \t\treturn -1;\n> \t} else if (mkdir(git_path_seq_dir(), 0777) < 0)\n> \t\treturn error_errno(_(\"could not create sequencer directory '%s'\"),\n> \n\n"},{"id":"376892","messageId":"20190609200038.GD28007@hank.intra.tgummerer.com","threadId":"51269","inReplyTo":"CAL7ArXpMBXwOGXWmYx8Rv9bj2trJ-FS_VGgPmJbAh+evbaxNEw@mail.gmail.com","subject":"Re: [GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commits","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-06-09T20:00:39Z","receivedAt":"2019-06-09T20:00:49Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 06/09, Rohit Ashiwal wrote:\n> > > PR: https://github.com/r1walz/git/pull/1\n> > > Reviewed-by: Elijah Newren \n> > > Reviewed-by: Thomas Gummerer \n> >\n> > Note that the 'Reviewed-by' footer is something that is \"given\" by the\n> > reviewers, and should only be added after they have explicitly given\n> > it for the patch/series in question. Instead this should probably\n> > mention that Elijah and me reviewed this off-list in the PR you linked\n> > to above.\n> \n> Oh! But I read here, SubmittingPatches[1] docs, that it's nice to give\n> credits to those who helped you. Should we fix it in some way? I don't\n> know. Anyway, that is beyond the scope of this patch.\n\nRight, thanks for thinking about credit here. The appropriate way to\ndo that would be to add a Helped-by trailer to the individual\ncommits, though especially for the first and last commit in the series\nwe probably don't deserve that much credit ;) In past GSoC/Outreachy\nprojects I've also seen the Mentored-by trailer.\n\nOther than Helped-by, which I've commonly used seen for this they do\nseem to be (more or less) documented in SubmittingPatches [2].\n\n> Thanks for the review\n> Rohit\n> \n> [1]: https://github.com/git/git/blob/v2.22.0/Documentation/SubmittingPatches#L289-L291\n\n[2]: https://github.com/git/git/blob/b697d92f56511e804b8ba20ccbe7bdc85dc66810/Documentation/SubmittingPatches#L353-L365\n"},{"id":"376908","messageId":"20190610051313.16222-1-rohit.ashiwal265@gmail.com","threadId":"51269","inReplyTo":"2007ef79-1791-4f77-f9e4-34c59b4f0c36@gmail.com","subject":"Re: [GSoC][PATCH 1/3] sequencer: add advice for revert","fromName":"Rohit Ashiwal","fromEmail":"rohit.ashiwal265@gmail.com","sentAt":"2019-06-10T05:13:13Z","receivedAt":"2019-06-10T05:15:28Z","isPatch":true,"sender":{"key":"rohit.ashiwal265@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31043830?v=4"},"body":"Hi Philip\n\nOn 2019-06-09 17:52 UTC Phillip Wood wrote:\n>\n> Hi Rohit\n> \n> Congratulations on your first GSoC patch series!\n\nThank you very much :)\n\n> On 08/06/2019 20:19, Rohit Ashiwal wrote:\n> > [...]\n> > @@ -2655,6 +2655,7 @@ static int create_seq_dir(void)\n> > \tif (file_exists(git_path_seq_dir())) {\n> > \t\terror(_(\"a cherry-pick or revert is already in progress\"));\n> > \t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit | --abort)\\\"\"));\n> > +\t\tadvise(_(\"or \\\"git revert (--continue | --quit | --abort)\\\"\"));\n> \n> I agree that it's a good idea to add advice for revert as well, but it\n> would be better to call sequencer_get_last_command() to find out if\n> we're running cherry-pick or revert and tailor the advice appropriately.\n\nFirstly, signature of `create_seq_dir` doesn't allow us to call\n`sequencer_get_last_command()`. Changing that for the sake of a\nbetter error message is too much task for this patch as it is a\nsubject of discussion on its own. (Also changing signature only\nmakes sense if this patch series gets merged). FWIW, I think we\nshould left this to further discussions for now and decide what\nto do later on.\n\n> Best Wishes\n> \n> Phillip\n\nThanks\nRohit\n\n"},{"id":"376909","messageId":"20190610052823.17013-1-rohit.ashiwal265@gmail.com","threadId":"51269","inReplyTo":"61a8bfde-018c-5a69-5abd-89d4a332e39e@gmail.com","subject":"Re: [GSoC][PATCH 3/3] cherry-pick/revert: update hints","fromName":"Rohit Ashiwal","fromEmail":"rohit.ashiwal265@gmail.com","sentAt":"2019-06-10T05:28:23Z","receivedAt":"2019-06-10T05:30:35Z","isPatch":true,"sender":{"key":"rohit.ashiwal265@gmail.com","avatar":"https://avatars.githubusercontent.com/u/31043830?v=4"},"body":"Hey Phillip\n\nOn Sun, 9 Jun 2019 19:03:02 +0100 Phillip Wood wrote:\n> \n> Hi Rohit\n> \n> On 08/06/2019 20:19, Rohit Ashiwal wrote:\n> > [...]\n> > @@ -2654,8 +2654,8 @@ static int create_seq_dir(void)\n> > {\n> > \tif (file_exists(git_path_seq_dir())) {\n> > \t\terror(_(\"a cherry-pick or revert is already in progress\"));\n> > -\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit | --abort)\\\"\"));\n> > -\t\tadvise(_(\"or \\\"git revert (--continue | --quit | --abort)\\\"\"));\n> > +\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --skip | --quit | --abort)\\\"\"));\n> > +\t\tadvise(_(\"or \\\"git revert (--continue | --skip | --quit | --abort)\\\"\"));\n> \n> If the user has already committed the conflict resolution then we don't\n> want to recommend --skip as there is nothing to skip.\n\nI think it is more about suggesting what are all the possibilities\nyou can try and not about intelligently suggesting what you should\ndo. ofc, we can not use `revert --