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

Hi Alban

On 02/11/2018 16:26, Alban Gruin wrote:

Hi Phillip,

Le 02/11/2018 à 11:09, Phillip Wood a écrit :

quoted

quoted

quoted

quoted

+    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