Re: [PATCH 2/2] rebase --exec: make it work with --rebase-merges
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Aug 6, 2018, 09:36 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1808061134320.71@tvgsbejvaqbjf.bet>
- In-Reply-To
- <xmqqa7q3w61q.fsf@gitster-ct.c.googlers.com>
Hi Junio,
On Fri, 3 Aug 2018, Junio C Hamano wrote:
Show 23 quoted lines
> "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
Show 21 quoted lines
>
> > + 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;
> > }
>