Re: Heads up: rebase -i -p will be made sane again

From: Stephen Haberman <stephen@exigencecorp.com>
Date: Jan 27, 2009, 14:54 UTC
Message-ID: <20090127085418.e113ad5a.stephen@exigencecorp.com>
In-Reply-To: <alpine.DEB.1.00.0901271012550.14855@racer>

> Dear list,

Thanks for keeping me on the cc list--several of the later stages of cruft are my fault, so I don't know that I'll be able to help any more than commentary on the use cases I was trying to fulfill.

Show 24 quoted lines

> As for the design bug I want to fix: imagine this history:
> 
>   ------A
>  /     /
> /     /
> ---- B
> \     \
>  \     \
>   C-----D-----E = HEAD
> 
> A, C and D touch the same file, and A and D agree on the contents.
> 
> Now, rebase -p A does the following at the moment:
> 
>   ------A-----E' = HEAD
>  /     /
> /     /
> ---- B
> 
> In other words, C is truly forgotten, and it is pretended that D never 
> happened, either.  That is exactly what test case 2 in t3410 tests for 
> [*1*].
> 
> This is insane.

Agreed.

Does this mean you're just getting rid of the code that calls "rev list --cherry-pick"?

If so, I'd be all for that--I did not introduce it, nor fully understand its nuances, and t3410 was just a hack to get the behavior of a rebase with a dropped/cherry picked commit from the previous behavior of being a no-op to instead do "something".

A few times I've pondered just removing the --cherry-pick/drop commit part of rebase-p, but assumed it was there for a reason.

Also, yeah, don't treat the test cases in t3410 as "the result should be this exact DAG" but "the result should be something that is not a noop/sane".

Show 8 quoted lines

> [*1*] The code in t3410 was not really easy to read, even if there was an 
> explanation what it tried to do, but the test code was inconsitent, 
> sometimes tagging, sometimes not, sometimes committing with -a, sometimes 
> "git add"ing first, yet almost repetitive.
> 
> In my endeavor not only to understand it, and either fix my code or the 
> code in t3410, I refactored it so that others should have a much easier 
> time to understand what it actually does.

Thanks for cleaning it up.

I recently saw a test of yours use a `test_commit` bash function that I really like. My last patch submission debacle had a patch cleaning up t3411 by introducing `test_commit`--I can brave `git send-email` again if you have any interest in me resending it.

Thanks, Stephen