[PATCH 2 of 3] improve pre-push logic to handle named branches better (issue736)

[Henrik Stuart]

Dirkjan Ochtman wrote:
> Some more nits...
[snip]
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -1432,42 +1432,83 @@
>>           else:
>>               bases, heads = update, self.changelog.heads()
>>
>> +        def checkbranch(lheads, rheads, updatelh):
>> +            warn = 0
>> +
>> +            if not revs and len(lheads)>  len(rheads):

[snip]

>> +                warn = 1
>> +            else:
>> +                newheads = set(sum([self.changelog.heads(x, lheads) \
>> +                                    for x in updatelh], []))
>> +                newheads&= set(lheads)
> 
> That does't make sense to me... Why sum([blah], [])? Something like this 
> seems much easier to read.

Python lacks a flatten construct, sum([blah], []) works as a list
flattening, i.e. sum([[1,2], [3,4]], []) = [1,2,3,4]. Perhaps not the
most apparent construct, but it's basically a specialised fold.

[snip]

> Does a nested function really make sense here? Can we extract it? You'd 
> have ui, changelog, and revs as extra args, I guess, so it may not be a 
> good trade-off. It should probably get a good docstring either way.

The function is only really relevant for the prepush logic, so I would
personally be a tad adverse to bringing it out to class scope as it is.

-- 
Kind regards,
  Henrik Stuart