[PATCH V2] rebase: do not raise an UnboundLocalError when called with wrong rev (issue4106)

[Simon Heimberg]

I will send V3 soon. More details below.

On 2014-02-19 20:55, David Soria Parra wrote:
> On 2/19/14, 2:55 AM, "Simon Heimberg" <simohe at besonet.ch> wrote:
>> diff -r 0e2877f8605d -r 2554f90793b6 hgext/rebase.py
>> --- a/hgext/rebase.py	Sat Feb 15 22:09:32 2014 -0600
>> +++ b/hgext/rebase.py	Fri Feb 14 00:34:20 2014 +0100
>> @@ -513,6 +513,11 @@
>>              if state.get(p.rev()) == repo[p1].rev():
>>                  base = p.node()
>>                  break
>> +        else:
>> +            # fallback when base not found (unnormal, see issue 4106)
>> +            #
>> +            # Raise because this function is (probaby) called wrong
>> +            raise AssertionError('function is called wrong')
> 1. I think we could do better as Œfunction is called wrong¹. Maybe Œbase
> must be defined¹.
We can not mention the variable "base", the caller can not define it. 
Maybe "no base found to rebase on (rebasenode called wrong)"?
> 2. I personally would have an invariant before the
>   assert Œbase¹ in locals()
>   if base is not None:
> making it clear that base needs to be defined after the whole if-else
> statement.
??
We can add "base = None" in else, it is always undefined there. Then we 
could just run on, the problem of not finding a valid base would be 
reported later (somehow).
By raising an AssertionError, we make clear that the caller of this 
function did something wrong. (And raising AssertionError is never 
optimized away, unlike assert.) The patch description tells this case 
only happens when an extension calls rebasenode() wrong.
> - David