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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

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

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> 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;
> >  		}
>