[PATCH] am: support --quit

[PATCH] am: support --quit

[Nguyễn Thái Ngọc Duy]

Among the "in progress" commands, only git-am and git-merge do not
support --quit. Support --quit in git-am too.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-am.txt               |  6 +++++-
 builtin/am.c                           | 12 ++++++++++--
 contrib/completion/git-completion.bash |  2 +-
 t/t4150-am.sh                          | 12 ++++++++++++
 4 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e4029..460662e4b9 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 	 [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
 	 [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]
 	 [(<mbox> | <Maildir>)...]
-'git am' (--continue | --skip | --abort)
+'git am' (--continue | --skip | --abort | --quit)
 
 DESCRIPTION
 -----------
@@ -167,6 +167,10 @@ default.   You can use `--no-utf8` to override this.
 --abort::
 	Restore the original branch and abort the patching operation.
 
+--quit::
+	Abort the patching operation but keep HEAD and the index
+	untouched.
+
 DISCUSSION
 ----------
 
diff --git a/builtin/am.c b/builtin/am.c
index 5bdd2d7578..793c1e2765 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2149,7 +2149,8 @@ enum resume_mode {
 	RESUME_APPLY,
 	RESUME_RESOLVED,
 	RESUME_SKIP,
-	RESUME_ABORT
+	RESUME_ABORT,
+	RESUME_QUIT
 };
 
 static int git_am_config(const char *k, const char *v, void *cb)
@@ -2249,6 +2250,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE(0, "abort", &resume,
 			N_("restore the original branch and abort the patching operation."),
 			RESUME_ABORT),
+		OPT_CMDMODE(0, "quit", &resume,
+			N_("abort the patching operation but keep HEAD where it is."),
+			RESUME_QUIT),
 		OPT_BOOL(0, "committer-date-is-author-date",
 			&state.committer_date_is_author_date,
 			N_("lie about committer date")),
@@ -2317,7 +2321,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		 * stray directories.
 		 */
 		if (file_exists(state.dir) && !state.rebasing) {
-			if (resume == RESUME_ABORT) {
+			if (resume == RESUME_ABORT || resume == RESUME_QUIT) {
 				am_destroy(&state);
 				am_state_release(&state);
 				return 0;
@@ -2359,6 +2363,10 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	case RESUME_ABORT:
 		am_abort(&state);
 		break;
+	case RESUME_QUIT:
+		am_rerere_clear();
+		am_destroy(&state);
+		break;
 	default:
 		die("BUG: invalid resume value");
 	}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 88813e9124..c7d5c7af29 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1077,7 +1077,7 @@ _git_am ()
 {
 	__git_find_repo_path
 	if [ -d "$__git_repo_path"/rebase-apply ]; then
-		__gitcomp "--skip --continue --resolved --abort"
+		__gitcomp "--skip --continue --resolved --abort --quit"
 		return
 	fi
 	case "$cur" in
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 73b67b4280..512c754e02 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -1045,4 +1045,16 @@ test_expect_success 'am works with multi-line in-body headers' '
 	git cat-file commit HEAD | grep "^$LONG$"
 '
 
+test_expect_success 'am --quit keeps HEAD where it is' '
+	mkdir .git/rebase-apply &&
+	>.git/rebase-apply/last &&
+	>.git/rebase-apply/next &&
+	git rev-parse HEAD^ >.git/ORIG_HEAD &&
+	git rev-parse HEAD >expected &&
+	git am --quit &&
+	test_path_is_missing .git/rebase-apply &&
+	git rev-parse HEAD >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
2.16.1.435.g8f24da2e1a

Re: [PATCH] am: support --quit

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Among the "in progress" commands, only git-am and git-merge do not
> support --quit. Support --quit in git-am too.

That's a strange way to phrase it, when the number of commands that
know and do not know it are about the same.

>  --abort::
>  	Restore the original branch and abort the patching operation.
>  
> +--quit::
> +	Abort the patching operation but keep HEAD and the index
> +	untouched.

OK, I see from the documentation of rebase that the above pair is
consistent with how --abort/--quit are done over there.

> @@ -2359,6 +2363,10 @@ int cmd_am(int argc, const char **argv, const char *prefix)
>  	case RESUME_ABORT:
>  		am_abort(&state);
>  		break;
> +	case RESUME_QUIT:
> +		am_rerere_clear();
> +		am_destroy(&state);
> +		break;

The internal implementation detail of am_abort() is leaking out
here, by saying "rerere-clear" is the only special thing other than
recovering the HEAD and working tree state when abort happens.  It
makes readers wonder if am_rerere_clear() should become part of
am_destroy().  I dunno.

Re: [PATCH] am: support --quit

[Duy Nguyen]

On Thu, Feb 15, 2018 at 2:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Among the "in progress" commands, only git-am and git-merge do not
>> support --quit. Support --quit in git-am too.
>
> That's a strange way to phrase it, when the number of commands that
> know and do not know it are about the same.

Arghh!! I thought I deleted that "git merge" part. That command is not
really in the same group as revert/rebase/cherry-pick/am. It only has
--continue, aborting is via "git reset"... probably because it's older
than most.

>> @@ -2359,6 +2363,10 @@ int cmd_am(int argc, const char **argv, const char *prefix)
>>       case RESUME_ABORT:
>>               am_abort(&state);
>>               break;
>> +     case RESUME_QUIT:
>> +             am_rerere_clear();
>> +             am_destroy(&state);
>> +             break;
>
> The internal implementation detail of am_abort() is leaking out
> here, by saying "rerere-clear" is the only special thing other than
> recovering the HEAD and working tree state when abort happens.  It
> makes readers wonder if am_rerere_clear() should become part of
> am_destroy().  I dunno.

I think the original design is am_destroy takes care of
$GIT_DIR/rebase-apply and nothing else. --abort has to clean up things
outside (index, HEAD, rerere) while a successful operation should not
leave anything else to clean up (except rebase-apply dir).
-- 
Duy

Re: [PATCH] am: support --quit

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> The internal implementation detail of am_abort() is leaking out
>> here, by saying "rerere-clear" is the only special thing other than
>> recovering the HEAD and working tree state when abort happens.  It
>> makes readers wonder if am_rerere_clear() should become part of
>> am_destroy().  I dunno.
>
> I think the original design is am_destroy takes care of
> $GIT_DIR/rebase-apply and nothing else. --abort has to clean up things
> outside (index, HEAD, rerere) while a successful operation should not
> leave anything else to clean up (except rebase-apply dir).

Yes, and that is why I think the code would have been nicer to
understand if the update to add 'reset' had turned the existing
am_abort() into a helper that is one level higher in the abstraction
(perhaps even by renaming the function) that the caller can tell
which part to clear (i.e. e.g. am_finish(&state, AM_CLEAR_ALL) vs
am_finish(&state, AM_CLEAR_STEP_ONLY)).  Stepping back even further,
perhaps the call made to am_destroy() in the normal exit case at the
end of am_run() could have been using the same helper.