Re: [PATCH/RFC/GSoC 12/17] rebase-todo: introduce rebase_todo_item

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

Hi Paul,

On Wed, 16 Mar 2016, Paul Tan wrote:

> On Mon, Mar 14, 2016 at 9:43 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
> > On Sat, Mar 12, 2016 at 11:46 AM, Paul Tan <pyokagan@gmail.com> wrote:
> >> In an interactive rebase, commands are read and executed from a todo
> >> list (.git/rebase-merge/git-rebase-todo) to perform the rebase.
> >>
> >> In the upcoming re-implementation of git-rebase -i in C, it is useful
> >> to be able to parse each command into a data structure which can then
> >> be operated on. Implement rebase_todo_item for this.
> >
> > sequencer.{c,h} already has some code to parse and create todo lists
> > for cherry-picking or reverting multiple commits, so I am wondering if
> > it would be possible to share some code?
> 
> AFAIK, sequencer.c as it is in master parses the todo list
> destructively and does not keep the associated action for each commit
> and the "rest" string.

This is a *serious* mistake in the implementation of the sequencer, I
agree.

Therefore it is a good idea to fix that mistake instead of leaving it in
place.

And that is exactly what I did:

	https://github.com/dscho/git/commit/b9b5b7351

Please note that the commit is marked as a "TODO" because it has to
reintroduce a stupidly strict behavior (the sequencer expects the commands
in the todo script to agree with the overall action, i.e. if the action is
REPLAY_REVERT, the todo script can only contain "revert" commands, if the
action is REPLAY_PICK, the todo list can only contain "pick" commands). Of
course this restriction is totally arbitrary and even unwanted, so I will
lift it after this commit. Or maybe I'll just lift it before this commit.
Yeah, I'll do that instead.

> As I said in another thread, originally I wanted to keep the scope
> simple, and just do the rewrite of rebase from C to shell, and let any
> further libifications and optimizations come after, so I didn't want
> to touch sequencer for now.

We know, however, how leaving technical debt for later will just make sure
that technical debt accumulates...

And while I have a *tremendous* respect for what Karthik did (and
continues to do, even long after his GSoC project ended!), I do not want
to ask any future GSoC student to clean up the mess that we produce right
now... So let's just not add more technical debt than we absolutely have
to.

Ciao,
Dscho