Re: [PATCH v2 06/16] sequencer: refactor sequencer_add_exec_commands() to work on a todo_list

[Phillip Wood <phillip.wood@talktalk.net>]

Hi Alban

On 02/11/2018 16:26, Alban Gruin wrote:
> 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.

I think in the last 'what's cooking' email Junio said he was going to 
merge all the builtin/rebase.c stuff to master so there may not be a 
problem if you wait a couple of days.

Best Wishes

Phillip

> Cheers,
> Alban
>