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

Le 01/02/2019 à 15:51, Phillip Wood a écrit :

quoted

quoted

@@ -4473,38 +4475,62 @@ int sequencer_add_exec_commands(struct

repository *r,
      * those chains if there are any.
      */
     insert = -1;
-    for (i = 0; i < todo_list.nr; i++) {
-        enum todo_command command = todo_list.items[i].command;
-
-        if (insert >= 0) {
-            /* skip fixup/squash chains */
-            if (command == TODO_COMMENT)
-                continue;
-            else if (is_fixup(command)) {
-                insert = i + 1;
-                continue;
-            }
-            strbuf_insert(buf,
-                      todo_list.items[insert].offset_in_buf +
-                      offset, commands, commands_len);

In a todo list that looks like
pick abc message
#pick cde empty commit
This inserts the exec command for the first pick above the commented
out pick. I think your translation puts it below the commented out
pick as it ignores the value of insert. I think it's probably easiest
to add an INSERT_ARRAY macro to insert it in the right place. An
alternative might be to track the last insert position and only copy
commands across when there is another exec to insert but that might
get complicated in cases such as

pick abc message
#squash cde squash! message //empty commit for rewording
fixup 123 fixup! message
#pick 456 empty commit

Thinking about this, I'm not sure it can happen as the empty squash
commit will be commented out before rearrange_squash() is called so I
think it would actually look like

pick abc message
fixup 123 fixup! message
#pick cde squash! message
#pick 456 empty commit

So I wonder if we can get away with treating insert as a flag and
inserting exec commands when
insert && command == TODO_COMMENT

We could probably do with some tests for this to be sure.

I will try this approach.

-- Alban

Best Wishes

Phillip