Re: [PATCH] am: support --quit

From: Duy Nguyen <pclouds@gmail.com>
Date: Feb 23, 2018, 10:24 UTC
Message-ID: <CACsJy8CUmpXNZDR6jvXP=hQkuTmCqzYYqitXJyYYR31Yw7DXXw@mail.gmail.com>
In-Reply-To: <xmqqy3jvmkyn.fsf@gitster-ct.c.googlers.com>

On Thu, Feb 15, 2018 at 2:26 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

> 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.

Show 14 quoted lines

>> @@ -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