{"thread":{"id":"47840","subject":"[PATCH] am: support --quit","startedAt":"2018-02-14T11:16:23Z","lastAt":"2018-02-23T18:00:24Z","messageCount":4,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"339271","messageId":"20180214111606.4257-1-pclouds@gmail.com","threadId":"47840","inReplyTo":null,"subject":"[PATCH] am: support --quit","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-02-14T11:16:06Z","receivedAt":"2018-02-14T11:16:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Among the \"in progress\" commands, only git-am and git-merge do not\nsupport --quit. Support --quit in git-am too.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/git-am.txt | 6 +++++-\n builtin/am.c | 12 ++++++++++--\n contrib/completion/git-completion.bash | 2 +-\n t/t4150-am.sh | 12 ++++++++++++\n 4 files changed, 28 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex 12879e4029..460662e4b9 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -16,7 +16,7 @@ SYNOPSIS\n \t [--exclude=] [--include=] [--reject] [-q | --quiet]\n \t [--[no-]scissors] [-S[]] [--patch-format=]\n \t [( | )...]\n-'git am' (--continue | --skip | --abort)\n+'git am' (--continue | --skip | --abort | --quit)\n \n DESCRIPTION\n -----------\n@@ -167,6 +167,10 @@ default. You can use `--no-utf8` to override this.\n --abort::\n \tRestore the original branch and abort the patching operation.\n \n+--quit::\n+\tAbort the patching operation but keep HEAD and the index\n+\tuntouched.\n+\n DISCUSSION\n ----------\n \ndiff --git a/builtin/am.c b/builtin/am.c\nindex 5bdd2d7578..793c1e2765 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -2149,7 +2149,8 @@ enum resume_mode {\n \tRESUME_APPLY,\n \tRESUME_RESOLVED,\n \tRESUME_SKIP,\n-\tRESUME_ABORT\n+\tRESUME_ABORT,\n+\tRESUME_QUIT\n };\n \n static int git_am_config(const char *k, const char *v, void *cb)\n@@ -2249,6 +2250,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\tOPT_CMDMODE(0, \"abort\", &resume,\n \t\t\tN_(\"restore the original branch and abort the patching operation.\"),\n \t\t\tRESUME_ABORT),\n+\t\tOPT_CMDMODE(0, \"quit\", &resume,\n+\t\t\tN_(\"abort the patching operation but keep HEAD where it is.\"),\n+\t\t\tRESUME_QUIT),\n \t\tOPT_BOOL(0, \"committer-date-is-author-date\",\n \t\t\t&state.committer_date_is_author_date,\n \t\t\tN_(\"lie about committer date\")),\n@@ -2317,7 +2321,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\t * stray directories.\n \t\t */\n \t\tif (file_exists(state.dir) && !state.rebasing) {\n-\t\t\tif (resume == RESUME_ABORT) {\n+\t\t\tif (resume == RESUME_ABORT || resume == RESUME_QUIT) {\n \t\t\t\tam_destroy(&state);\n \t\t\t\tam_state_release(&state);\n \t\t\t\treturn 0;\n@@ -2359,6 +2363,10 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \tcase RESUME_ABORT:\n \t\tam_abort(&state);\n \t\tbreak;\n+\tcase RESUME_QUIT:\n+\t\tam_rerere_clear();\n+\t\tam_destroy(&state);\n+\t\tbreak;\n \tdefault:\n \t\tdie(\"BUG: invalid resume value\");\n \t}\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 88813e9124..c7d5c7af29 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1077,7 +1077,7 @@ _git_am ()\n {\n \t__git_find_repo_path\n \tif [ -d \"$__git_repo_path\"/rebase-apply ]; then\n-\t\t__gitcomp \"--skip --continue --resolved --abort\"\n+\t\t__gitcomp \"--skip --continue --resolved --abort --quit\"\n \t\treturn\n \tfi\n \tcase \"$cur\" in\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 73b67b4280..512c754e02 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -1045,4 +1045,16 @@ test_expect_success 'am works with multi-line in-body headers' '\n \tgit cat-file commit HEAD | grep \"^$LONG$\"\n '\n \n+test_expect_success 'am --quit keeps HEAD where it is' '\n+\tmkdir .git/rebase-apply &&\n+\t>.git/rebase-apply/last &&\n+\t>.git/rebase-apply/next &&\n+\tgit rev-parse HEAD^ >.git/ORIG_HEAD &&\n+\tgit rev-parse HEAD >expected &&\n+\tgit am --quit &&\n+\ttest_path_is_missing .git/rebase-apply &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.16.1.435.g8f24da2e1a\n\n"},{"id":"339389","messageId":"xmqqy3jvmkyn.fsf@gitster-ct.c.googlers.com","threadId":"47840","inReplyTo":"20180214111606.4257-1-pclouds@gmail.com","subject":"Re: [PATCH] am: support --quit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-14T19:26:24Z","receivedAt":"2018-02-14T19:26:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Among the \"in progress\" commands, only git-am and git-merge do not\n> support --quit. Support --quit in git-am too.\n\nThat's a strange way to phrase it, when the number of commands that\nknow and do not know it are about the same.\n\n> --abort::\n> \tRestore the original branch and abort the patching operation.\n> \n> +--quit::\n> +\tAbort the patching operation but keep HEAD and the index\n> +\tuntouched.\n\nOK, I see from the documentation of rebase that the above pair is\nconsistent with how --abort/--quit are done over there.\n\n> @@ -2359,6 +2363,10 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n> \tcase RESUME_ABORT:\n> \t\tam_abort(&state);\n> \t\tbreak;\n> +\tcase RESUME_QUIT:\n> +\t\tam_rerere_clear();\n> +\t\tam_destroy(&state);\n> +\t\tbreak;\n\nThe internal implementation detail of am_abort() is leaking out\nhere, by saying \"rerere-clear\" is the only special thing other than\nrecovering the HEAD and working tree state when abort happens. It\nmakes readers wonder if am_rerere_clear() should become part of\nam_destroy(). I dunno.\n"},{"id":"340010","messageId":"CACsJy8CUmpXNZDR6jvXP=hQkuTmCqzYYqitXJyYYR31Yw7DXXw@mail.gmail.com","threadId":"47840","inReplyTo":"xmqqy3jvmkyn.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] am: support --quit","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-02-23T10:24:34Z","receivedAt":"2018-02-23T10:25:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Feb 15, 2018 at 2:26 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> Among the \"in progress\" commands, only git-am and git-merge do not\n>> support --quit. Support --quit in git-am too.\n>\n> That's a strange way to phrase it, when the number of commands that\n> know and do not know it are about the same.\n\nArghh!! I thought I deleted that \"git merge\" part. That command is not\nreally in the same group as revert/rebase/cherry-pick/am. It only has\n--continue, aborting is via \"git reset\"... probably because it's older\nthan most.\n\n>> @@ -2359,6 +2363,10 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n>> case RESUME_ABORT:\n>> am_abort(&state);\n>> break;\n>> + case RESUME_QUIT:\n>> + am_rerere_clear();\n>> + am_destroy(&state);\n>> + break;\n>\n> The internal implementation detail of am_abort() is leaking out\n> here, by saying \"rerere-clear\" is the only special thing other than\n> recovering the HEAD and working tree state when abort happens. It\n> makes readers wonder if am_rerere_clear() should become part of\n> am_destroy(). I dunno.\n\nI think the original design is am_destroy takes care of\n$GIT_DIR/rebase-apply and nothing else. --abort has to clean up things\noutside (index, HEAD, rerere) while a successful operation should not\nleave anything else to clean up (except rebase-apply dir).\n-- \nDuy\n"},{"id":"340049","messageId":"xmqq606nvb68.fsf@gitster-ct.c.googlers.com","threadId":"47840","inReplyTo":"CACsJy8CUmpXNZDR6jvXP=hQkuTmCqzYYqitXJyYYR31Yw7DXXw@mail.gmail.com","subject":"Re: [PATCH] am: support --quit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-23T18:00:15Z","receivedAt":"2018-02-23T18:00:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n>> The internal implementation detail of am_abort() is leaking out\n>> here, by saying \"rerere-clear\" is the only special thing other than\n>> recovering the HEAD and working tree state when abort happens. It\n>> makes readers wonder if am_rerere_clear() should become part of\n>> am_destroy(). I dunno.\n>\n> I think the original design is am_destroy takes care of\n> $GIT_DIR/rebase-apply and nothing else. --abort has to clean up things\n> outside (index, HEAD, rerere) while a successful operation should not\n> leave anything else to clean up (except rebase-apply dir).\n\nYes, and that is why I think the code would have been nicer to\nunderstand if the update to add 'reset' had turned the existing\nam_abort() into a helper that is one level higher in the abstraction\n(perhaps even by renaming the function) that the caller can tell\nwhich part to clear (i.e. e.g. am_finish(&state, AM_CLEAR_ALL) vs\nam_finish(&state, AM_CLEAR_STEP_ONLY)). Stepping back even further,\nperhaps the call made to am_destroy() in the normal exit case at the\nend of am_run() could have been using the same helper.\n\n\n\n"}]}