[PATCH 4 of 5 v3] rebase: refactor computerebase to case-by-case handling

[Matt Mackall]

On Thu, 2014-12-04 at 04:12 +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski at unity3d.com>
> # Date 1416364731 -3600
> #      Wed Nov 19 03:38:51 2014 +0100
> # Node ID 742988c7a05cb1bd49f9be0f057a9c8ef4e20d95
> # Parent  e031c5201577b359af981396957a0b9a9a5f5300
> rebase: refactor computerebase to case-by-case handling
> 
> This makes it simpler to review and tweak handling of different cases.
> 
> This implementation was created using the old implementation as black box. It
> started out with how I think it should be, and was tweaked until it matched how
> it actually is. Further tweaking can take us close to where we want to be, step
> by step.
> 
> The only case where the test suite coverage gives different result from
> computerebase is in test-rebase-obsolete.t in a case where None as ancestor
> would give the same ancestor as the one previously explicitly specified.
> 
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -25,6 +25,7 @@ import os, errno
>  
>  nullmerge = -2
>  revignored = -3
> +outside = -4 # in computerebase: not in state, thus outside rebase set
>  
>  cmdtable = {}
>  command = cmdutil.command(cmdtable)
> @@ -586,88 +587,93 @@ def computerebase(repo, rev, target, sta
>      '''Return the merge revisions and new parent relationship of rev rebased
>      to target:
>          merge parent 1, merge parent 2, ancestor, new parent 1, new parent 2
> +
> +    The following table shows how it is - not necessarily how it should be.
> +
> +    Rebasing of rev with parents p1 and p2 onto target as rev' is written:
> +        m1,m2|a
> +        p1',p2'
> +
> +        \  p2     null         ancestor        rebased        outside
> +      p1 \
> +    null       target,rev|  target,rev|None  p2',rev|p2    target,rev|p2
> +                 target         target           p2'          target
> +
> +    ancestor target,rev|Non target,rev|None  p2',rev|p2   target,rev|None
> +                 target         target           p2'         target,p2
> +
> +    rebased    p1',rev|p1     p1',rev|p1    target,rev|p1   p1',rev|p1
> +                   p1'            p1'          p1',p2         p1',p2
> +
> +    outside   target,rev|p1 target,rev|None  p2',rev|p2       abort
> +                 target        target,p1       p2',p1       3 parents

Woo, a table! If I read your commit message right, this table matches
the behavior of both the old and new code, right? If so, I'd like to get
some form of this table (and your docstring) in place as a first step.
Last time I looked at this code, I really struggled to figure out what
the args to this function meant or even what types they were.

I think we need a key:

p1' = already rebased rev of p1
p2' = already rebased rev of p2
ancestor = rev is in targetancestors
rebased = rev has been rebased
outside = other

Is there a difference between null(rev) and None?

-- 
Mathematics is the supreme nostalgia of our time.