Re: [PATCH v2 01/34] sequencer: support a new action: 'interactive rebase'

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 19, 2016, 13:33 UTC
Message-ID: <alpine.DEB.2.20.1612191408250.54750@virtualbox>
In-Reply-To: <xmqqk2b31sfs.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Tue, 13 Dec 2016, Junio C Hamano wrote:

Show 18 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > @@ -395,7 +414,10 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
> >  
> >  	if (active_cache_changed &&
> >  	    write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
> > -		/* TRANSLATORS: %s will be "revert" or "cherry-pick" */
> > +		/*
> > +		 * TRANSLATORS: %s will be "revert", "cherry-pick" or
> > +		 * "rebase -i".
> > +		 */
> 
> IIRC, the "TRANSLATORS:" comment has to deviate from our coding
> style due to tool limitation and has to be done like this:
> 
> > +		/* TRANSLATORS: %s will be "revert", "cherry-pick" or
> > +		 * "rebase -i".
> > +		 */

Aargh. I had fixed that in another patch series already, but failed to do so in this here patch series. Sorry.

Fixed.

Show 10 quoted lines

> > @@ -1204,6 +1226,9 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
> >  	const char *todo_path = get_todo_path(opts);
> >  	int next = todo_list->current, offset, fd;
> >  
> > +	if (is_rebase_i(opts))
> > +		next++;
> > +
> 
> This is because...?  Everybody else counts 0-based while rebase-i
> counts from 1 or something?

No. Since its conception, edit-patch-series.sh (the artist now known as rebase -i) processed each line of the "todo script" by writing a new "todo" file, skipping the first line, and appending it to the "done" file.

The sequencer chooses to write the new "insn sheet" after a successful operation.

This is not an option for rebase -i, as it has many error paths and it was simply impossible to express "save the todo script in case of failure" in shell script.

I added a comment to clarify why the current command is not written to git-rebase-todo.

Ciao, Dscho