From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Tue, 12 Jul 2011 06:26:14 GMT
Subject: Re: [PATCH 15/17] revert: Remove sequencer state when no commits are pending
Message-ID: <CALkWK0=5ExB0a_VKY=F6m-q4rx0F-uzCgGywj5zsgY5qWxVZjA@mail.gmail.com>
In-Reply-To: <7vvcv8bjmf.fsf@alter.siamese.dyndns.org>

Hi Junio,

Junio C Hamano wites:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> diff --git a/builtin/revert.c b/builtin/revert.c
>> index f9f5e3a..3936516 100644
>> --- a/builtin/revert.c
>> +++ b/builtin/revert.c
>> @@ -990,8 +990,18 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
>>       for (cur = todo_list; cur; cur = cur->next) {
>>               save_todo(cur, opts);
>>               res = do_pick_commit(cur->item, opts);
>> -             if (res)
>> +             if (res) {
>> +                     if (!cur->next)
>> +                             /*
>> +                              * An error was encountered while
>> +                              * picking the last commit; the
>> +                              * sequencer state is useless now --
>> +                              * the user simply needs to resolve
>> +                              * the conflict and commit
>> +                              */
>> +                             remove_sequencer_state();
>>                       return res;
>> +             }
>>       }
>
> It may be useless for --continue, but wouldn't --abort need some clue on
> what you were doing?

Jonathan pointed this out too.  I'd like to know what you think of the
alternatives
1. Layering a hack on top: writing .git/sequencer/to-remove or
something as ugly.
2. Teaching "git commit" to remove the sequencer state instead.

Actually, an "--abort" can be taught to work even here by reading
.git/sequencer-old/head, but that's arguably ugly too.

-- Ram