From: Alban Gruin <alban.gruin@gmail.com>
Date: Fri, 02 Nov 2018 16:26:53 GMT
Subject: Re: [PATCH v2 06/16] sequencer: refactor sequencer_add_exec_commands() to work on a todo_list
Message-ID: <878848f4-f8f6-25cb-061e-5f7aef9ca1a9@gmail.com>
In-Reply-To: <351e3398-8239-5348-2880-d253d8a125d7@talktalk.net>

Hi Phillip,

Le 02/11/2018 à 11:09, Phillip Wood a écrit :
>>>> +    struct todo_item *items = NULL,
>>>> +        base_item = {TODO_EXEC, NULL, 0, 0, commands_len, 0};
>>>> +
>>>> +    strbuf_addstr(buf, commands);
>>>> +    base_item.offset_in_buf = buf->len - commands_len - 1;
>>>> +    base_item.arg = buf->buf + base_item.offset_in_buf;
>>>
>>> I think if the user gives --exec more than once on the command line then
>>> commands will contain more than one exec command so this needs to parse
>>> commands and create one todo_item for each command.
>>>
>>
>> Ouch, you’re right.  Thanks for the heads up.
> 
> I haven't looked how difficult it would be but it might be best to
> change the option parsing to pass an array of strings containing the
> exec commands rather than one long string so we can just loop over the
> array here.
> 

It would be the best way to do so.  This string comes from git-rebase.sh
(or builtin/rebase.c) -- they format it this way before invoking
git-rebase--interactive.  So either I modify both of them (for this, I
would need to rebase my branch on master), or I can split this string in
builtin/rebase--interactive.c.  I prefer the first option, but maybe
changing the base of this series will not please Junio.

Cheers,
Alban