[PATCH] context: follow all branches in blockdescendants()

[Denis Laxalde]

Yuya Nishihara a écrit :
> On Fri, 14 Apr 2017 08:57:21 +0200, Denis Laxalde wrote:
>> # HG changeset patch
>> # User Denis Laxalde <denis at laxalde.org>
>> # Date 1492152918 -7200
>> #      Fri Apr 14 08:55:18 2017 +0200
>> # Node ID 0bed2dff50d5bc32820c973b7057f32dd97dd89d
>> # Parent  f23d579a5a04e44f5e0370ba13ad20dd626e8ad7
>> context: follow all branches in blockdescendants()
>
> Looks better, queued, thanks.
>
> A couple of comments follow.
>
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -1216,17 +1216,21 @@ def blockdescendants(fctx, fromline, tol
>>      fl = fctx.filelog()
>>      seen = {fctx.filerev(): (fctx, (fromline, toline))}
>
> Perhaps the start revision itself should be yielded if the file is changed
> in that revision?

Yes, that'd make sense since descendants() actually does that (didn't
noticed before).

>>      for i in fl.descendants([fctx.filerev()]):
>> +        if i in seen:
>> +            continue
>
> I might be wrong, but I'm not sure how this would happen.

Yes, that's right. It does not happen in tests at least.

>>          c = fctx.filectx(i)
>> +        inrange = False
>>          for x in fl.parentrevs(i):
>>              try:
>> -                p, linerange2 = seen.pop(x)
>> +                p, linerange2 = seen[x]
>>              except KeyError:
>>                  # nullrev or other branch
>>                  continue
>> -            inrange, linerange1 = _changesrange(c, p, linerange2, diffopts)
>> -            if inrange:
>> -                yield c, linerange1
>> +            inrangep, linerange1 = _changesrange(c, p, linerange2, diffopts)
>> +            inrange = inrange or inrangep
>>              seen[i] = c, linerange1
>
> Does this mean the merge revision 'i' may have two separate lineranges coming
> from 'x's, but one is lost?

Yes. I can add:

   assert i not in seen or seen[i] == (c, linerange1)

before overriding seen[i] (this happens with revision 20 in tests). If
the assertion fails (it doesn't in current tests), that would indicate a
problem in the algorithm instead of maybe producing wrong results. What
do you think?

>> +        if inrange:
>> +            yield c, linerange1