git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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;
> >  		}
> 
Previous: Junio C HamanoNext: Johannes Schindelin via GitGitGadget
Message 9 of 21 in “Make git rebase work with --rebase-merges and --exec”
  1. 0/2 Make git rebase work with --rebase-merges and --execJohannes Schindelin via GitGitGadget, Aug 3, 2018
  2. 1/2 t3430: demonstrate what -r, --autosquash & --exec should doJohannes Schindelin via GitGitGadget, Aug 3, 2018
  3. 2/2 rebase --exec: make it work with --rebase-mergesJohannes Schindelin via GitGitGadget, Aug 3, 2018
  4. Junio C HamanoAug 3, 2018
  5. Johannes SchindelinAug 6, 2018
  6. Johannes SchindelinAug 6, 2018
  7. Junio C HamanoAug 6, 2018
  8. Junio C HamanoAug 3, 2018
  9. Johannes SchindelinAug 6, 2018
  10. 0/2 Make git rebase work with --rebase-merges and --execJohannes Schindelin via GitGitGadget, Aug 6, 2018
  11. 1/2 t3430: demonstrate what -r, --autosquash & --exec should doJohannes Schindelin via GitGitGadget, Aug 6, 2018
  12. 2/2 rebase --exec: make it work with --rebase-mergesJohannes Schindelin via GitGitGadget, Aug 6, 2018
  13. Phillip WoodAug 6, 2018
  14. Phillip WoodAug 6, 2018
  15. Johannes SchindelinAug 9, 2018
  16. Phillip WoodAug 9, 2018
  17. Johannes SchindelinAug 9, 2018
  18. Johannes SchindelinAug 6, 2018
  19. 0/2 Make git rebase work with --rebase-merges and --execJohannes Schindelin via GitGitGadget, Aug 9, 2018
  20. 1/2 t3430: demonstrate what -r, --autosquash & --exec should doJohannes Schindelin via GitGitGadget, Aug 9, 2018
  21. 2/2 rebase --exec: make it work with --rebase-mergesJohannes Schindelin via GitGitGadget, Aug 9, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.