From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Fri, 09 Dec 2011 05:34:15 GMT
Subject: Re: [PATCH 4/5] revert: allow mixed pick and revert instructions
Message-ID: <CALkWK0=4Mzu57t3_CCTpbWrTFfHu3aF=5aNv-NqcmQ6BMG2tSg@mail.gmail.com>
In-Reply-To: <20111207074501.GE11737@elie.hsd1.il.comcast.net>

Hi,

Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> This patch lays the foundation for extending the parser to support
>> more actions so 'git rebase -i' can reuse this machinery in the
>> future.  While at it, also improve the error messages reported by the
>> insn sheet parser.
>
> I don't know what this part is about...

I'll separate out these changes in the re-roll.  Thanks.

>> @@ -733,37 +739,29 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
> [...]
>>       status = get_sha1(bol, commit_sha1);
>>       *end_of_object_name = saved;
> [...]
>>       if (status < 0)
>> -             return NULL;
>> +             return error(_("Line %d: Malformed object name: %s"), lineno, bol);
>
> (Not about this patch, but an earlier one)  What is this idiom about?
> Why not
>
>        if (get_sha1(bol, commit_sha1))
>                return error(_(...));
>
> ?

I'm first detecting the end of the object name and terminating it with
a '\0', before using using get_sha1() to read out the object name.
Then, I restore the instruction sheet to the original state by
restoring the character from 'saved'.  If I use the idiom you
suggested, I'll leave midway after editing the instruction sheet, and
this is undesirable.

-- Ram