Re: [PATCH 15/17] revert: Remove sequencer state when no commits are pending

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jul 12, 2011, 06:26 UTC
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:

Show 26 quoted lines

> 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