Hi Kuba & Junio,

On Wed, 31 Aug 2016, Jakub Narębski wrote:

> W dniu 31.08.2016 o 21:10, Junio C Hamano pisze:
> > Jakub Narębski <jnareb@gmail.com> writes:
> > 
> >>> diff --git a/sequencer.c b/sequencer.c
> >>> index 06759d4..3398774 100644
> >>> --- a/sequencer.c
> >>> +++ b/sequencer.c
> >>> @@ -709,6 +709,8 @@ static int read_and_refresh_cache(struct replay_opts *opts)
> >>>  struct todo_item {
> >>>  	enum todo_command command;
> >>>  	struct commit *commit;
> >>> +	const char *arg;
> >>> +	int arg_len;
>  
> > I am not sure what the "commit" field of type "struct commit *" is
> > for.  It is not needed until it is the commit's turn to be picked or
> > reverted; if we end up stopping in the middle, parsing the commit
> > object for later steps will end up being wasted effort.
> 
> From what I understand this was what sequencer did before this
> series, so it is not a regression (I think; the commit parsing
> was in different function, but I think at the same place in
> the callchain).

Exactly.

> > Also, when the sequencer becomes one sequencer to rule them all, the
> > command set may contain something that does not even mention any
> > commit at all (think: exec).
> 
> The "exec" line is a bit of exception, all other rebase -i commands
> take commit as parameter.  It could always use NULL.

There is also "noop".

> > So I am not sure if we want a parsed commit there (I would not
> > object if we kept the texual object name read from the file,
> > though).  The "one sequencer to rule them all" may even have to say
> > "now give name ':1' to the result of the previous operation" in one
> > step and in another later step have an instruction "merge ':1'".
> > When that happens, you cannot even pre-populate the commit object
> > when the sequencer reads the file, as the commit has not yet been
> > created at that point.
> 
> True, --preserve-merges rebase is well, different.

It is mis-designed. And I can be that harsh because it was my design.

In the meantime I came up with a much better design, and implemented it as
a shell script on top of rebase -i. Since shell scripts run like slow
molasses, even more so on Windows, I have a loose plan to implement its
functionality as a new --recreate-merges option, and to deprecate
--preserve-merges when that new option works.

It needs to be a new option (not a --preserve-merges=v2) because it is a
totally different beast. For starters, it does not need its own code path
that overrides pick_one, as --preserve-merges does.

But I get way ahead of myself. First we need to get these last few bits
and pieces in place to accelerate (non --preserve-merges) rebase -i.

Ciao,
Dscho