Re: [PATCH v2 2/2] rebase --exec: make it work with --rebase-merges

Hi Phillip,

On Thu, 9 Aug 2018, Phillip Wood wrote:

On 09/08/18 10:22, Johannes Schindelin wrote:

quoted

On Mon, 6 Aug 2018, Phillip Wood wrote:

quoted

On 06/08/18 10:52, Johannes Schindelin via GitGitGadget wrote:

quoted

+			else if (is_fixup(command)) {
+				insert = i + 1;
+				continue;
+			}
+			strbuf_insert(buf,
+				      todo_list.items[insert].offset_in_buf +
+				      offset, commands, commands_len);
  			offset += commands_len;
+			insert = -1;
  		}
-		first = 0;
+
+		if (command == TODO_PICK || command == TODO_MERGE)
+			insert = i + 1;
   }
  
  	/* append final <commands> */
-	strbuf_add(buf, commands, commands_len);
+	if (insert >= 0 || !offset)
+		strbuf_add(buf, commands, commands_len);

Having read your other message about this patch I think if you wanted to fix
the position of the final exec in the case where the todo list ends with a
comment you could do something like

	if (insert >= 0)
		strbuf_insert(buf,
			      todo_list.items[insert].offset_in_buf +
			      offset, commands, commands_len);
	else
		strbuf_add(buf, commands, commands_len);

That does not really work, as `insert` can point *after* the last line, in
which case `todo_list.items[insert]` is undefined (and in the worst case,
causes a segmentation fault).

Ah, I'd missed that, does changing the conditions to
if (insert >= 0 && insert < todo.list_nr) and
else if (insert >=0 || !offset) work?

That's pretty exactly what I did ;-)

Ciao,
Dscho