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

"Johannes Schindelin via GitGitGadget" [off-list ref]
writes:

+	/*
+	 * Insert <commands> after every pick. Here, fixup/squash chains
+	 * are considered part of the pick, so we insert the commands *after*
+	 * those chains if there are any.
+	 */
+	insert_final_commands = 1;
+	for (i = 0; i < todo_list.nr; ) {
+		enum todo_command command = todo_list.items[i].command;
+		int j = 0;
+ ...
+		/* skip fixup/squash chain, if any */
+		for (i++; i < todo_list.nr; i++, j = 0) {

Does 'j' need to be reset to 0 in each iteration?  Nobody looks at
'j' after exiting this inner loop, and every referernce to 'j'
inside this inner loop happens _after_ it gets assigned "i+1" at the
beginning of "skip comment" loop.

For that matter, I wonder if 'j' can be a variable local to this
inner loop, not the outer loop that iterates over todo_list.items[].

+			command = todo_list.items[i].command;
+
+			if (is_fixup(command))
+				continue;
+
+			if (command != TODO_COMMENT)
+				break;
+
+			/* skip comment if followed by any fixup/squash */
+			for (j = i + 1; j < todo_list.nr; j++)
+				if (todo_list.items[j].command != TODO_COMMENT)
+					break;
+			if (j < todo_list.nr &&
+			    is_fixup(todo_list.items[j].command)) {
+				i = j;
+				continue;
+			}
+			break;
 		}

Hi Junio,

On Fri, 3 Aug 2018, Junio C Hamano wrote:

"Johannes Schindelin via GitGitGadget" [off-list ref]
writes:

quoted

+	/*
+	 * Insert <commands> after every pick. Here, fixup/squash chains
+	 * are considered part of the pick, so we insert the commands *after*
+	 * those chains if there are any.
+	 */
+	insert_final_commands = 1;
+	for (i = 0; i < todo_list.nr; ) {
+		enum todo_command command = todo_list.items[i].command;
+		int j = 0;
+ ...
+		/* skip fixup/squash chain, if any */
+		for (i++; i < todo_list.nr; i++, j = 0) {

Does 'j' need to be reset to 0 in each iteration?  Nobody looks at
'j' after exiting this inner loop, and every referernce to 'j'
inside this inner loop happens _after_ it gets assigned "i+1" at the
beginning of "skip comment" loop.

For that matter, I wonder if 'j' can be a variable local to this
inner loop, not the outer loop that iterates over todo_list.items[].

I rewrote this code, and the `j` variable is not even there anymore.

Ciao,
Dscho

quoted

+			command = todo_list.items[i].command;
+
+			if (is_fixup(command))
+				continue;
+
+			if (command != TODO_COMMENT)
+				break;
+
+			/* skip comment if followed by any fixup/squash */
+			for (j = i + 1; j < todo_list.nr; j++)
+				if (todo_list.items[j].command != TODO_COMMENT)
+					break;
+			if (j < todo_list.nr &&
+			    is_fixup(todo_list.items[j].command)) {
+				i = j;
+				continue;
+			}
+			break;
 		}