[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    rebase -i's todo/done list, was Re: [PATCH] rebase -i: redo tasks that die during cherry-pick
From:       Johannes Schindelin <johannes.schindelin () gmx ! de>
Date:       2015-04-30 7:49:40
Message-ID: 66567923994761429d29715508bc415f () www ! dscho ! org
[Download RAW message or body]

Hi Junio,

On 2015-04-29 19:15, Junio C Hamano wrote:

> [I] wonder if it would have been a better design to have a static "todo" with a \
> "current" pointer as two state files.

I decided against this in my original design because I frequently edited the todo \
list during the rebase run, already back when I had still called it \
`edit-patch-series.sh` (you will remember that "Saturday project" I reported on IRC).

One example for such an editing happens to me quite frequently: I reorder patches \
into a more logical order, except that I make mistakes and try to pull a patch too \
early, i.e. before the code it touches was added. In such a case, I obviously get a \
merge conflict, and after analysing the problem I have a better idea where to put \
that patch, which I do by adding a "pick deadbeef" into the `git-rebase-todo` list \
(these days, I use `git rebase --edit-todo`, of course).

If the list and the "current" pointer would be two different things, it would be way \
too easy to incur inconsistencies. Even for the inventor of rebase -i.

> Then reschedule would have been just the matter of decrementing the number in \
> "current", instead of "grab the last line of one file and prepend to the other \
> file, and then lose the last line".

I think that is only an implementation detail: whether we decrement a pointer or put \
back a line we moved to another file is essentially the same operation: it is a \
roll-back. The "more invasive" change I implied was to *avoid* the need for a \
roll-back, i.e. only move that line (or in your example, increment the pointer) after \
the operation completed successfully.

My implied change would mean that all the `mark_action_done` calls would have to be \
moved just before the `record_in_rewritten` calls -- but not all actions have that \
call. The tricky bit is that there are side effects: `peek_next_command` relies on \
the current behavior and would have to be adjusted. And that is only the side effect \
that I can think of off the top of my head.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[prev in list] [next in list] [prev in thread] [next in thread]