Re: [RFC PATCH] rebase: implement --rewind

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Apr 05 2023, Oswald Buddenhagen wrote:

> On Wed, Apr 05, 2023 at 02:07:29PM +0200, Johannes Schindelin wrote:
>> This question brings me back to the initial question: What problem
>> do we try to solve here? (This is a question that try as I might, I
>> cannot see answered in the proposed commit message.)
>>
> and i, try as i might, don't understand what you're not understanding
> ...
>
>>[...] In other words, I need a nested rebase.
>>
> that's just *your* private terminology. i don't apply the term
> "nested" here, because for me that implies the possibility to
> "unnest", which my patch doesn't implement. instead, it just continues
> past the point where the rewind was initiated. it's the difference
> between a loop and recursion.
> but outside this difference in terminology, for all i can tell, my
> patch implements *exactly* what you're asking for, and i don't
> understand why that's not obvious to you, given how well you
> understand the problem space yourself.
> please describe what you want with _a few_ words and without
> introducing any new terminology first, i.e., something you'd actually
> want to see in the feature's summary documentation. that should
> illuminate what keywords you find critical.
>
> i just gave rewinding rebasing merges a shot, and it didn't work for
> the simple reason that --rebase-merges is not saved in the state
> (understandably, because that was unnecessary so far) and the
> combination of --rewind --rebase-merges is being rejected. i'll need
> to fix that.
>
> then there is the problem that --rebase-merges only redoes merges
> rather than replaying them. but it seems that the simple case with
> unmodified parents actually does get replayed (or rather, skipped
> over, just incredibly slowly), so rewinding to just the last merge
> would work fine.  other than that, i'm declaring the matter out of
> scope and deferring to your "replaying evil merges" sub-thread.

Not Johannes, but I'd also like to have "nested", but maybe your feature
would also provide that. I haven't had time to test it, sorry.

But isn't the difference noted in this aspect of your commit message:
"where one can return to the pre-rewind state even after committing the
todo edit".

My most common use-case for "nested" is certainly less complex that
Johannes's, and is the following:

 * I've got e.g. a 10 patch series

 * I start rebasing that on "master", solve conflicts with "1..4", and
   am now on a conflict on 5/10.

 * It now becomes obvious to me that the even larger conflict I'm about
   to have on 6/10 would be better handled if I went back to 2/10 or
   whatever, did a change I could do here in 5/10 differently, and then
   proceeded.

I.e. when I'm at 5/10 I'd conceptually like to do another "git rebase -i
HEAD~5" or whatever, use the *already rewritten* commits (otherwise I'd
just abort and restast), re-arrange/rewrite them, and when I'm done
return to 5/10.

Then do another "continue".

From a UX perspective I think just as our $PS1 integration can be made
to show "5/10" it would be ideal if in this case we could show
e.g. "5/10 -> 1/5" or whatever. I.e. I'm in a nested rebase of 1/5,
which started from that 5/10".

Right now I do this sort of thing manually, i.e. note the SHA-1's I've
got so far, --abort at 5/10, then start a rebase for all 10 again, but
manually replace the SHA-1's for 1-5 with the ones I had already.

Which, I suppose I could also do the other way around, i.e. at 5/10 I'd
--edit-todo, wipe away 6/10, "finish" my rebase, then use "git rebase
--onto" later when I'm done to transplant the remaining 6-10/10 on the
1-5/5 I'm now happy with.

But here's the important bit: Sometimes I'm just wrong about my re-edit
to 2/10 being the right thing, and it would actually just make things
worse, as I might discover in my "nested" rebase once I'm at 4/5 or
whatever.

So being able to do an "--abort" ot that point to go back to the
"un-nested" 5/10 (*not* "original" 5/10) and proceed from there would be
nice.

But I think what you've implemented doesn't do that at all, or am I
misunderstanding you?

I think a relatively simple hack to "restart" might still be very nice,
just clarifying.