Hi Junio,
On Fri, 3 Aug 2018, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> 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[].
I rewrote this code, and the `j` variable is not even there anymore.
Ciao,
Dscho
>
> > + 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;
> > }
>
