[PATCH 1 of 3] mdiff: add a "blocksinrange" function to filter diff blocks by line range

Denis Laxalde denis.laxalde at logilab.fr
Fri Dec 2 02:32:03 EST 2016


Yuya Nishihara a écrit :
> On Mon, 28 Nov 2016 10:54:14 +0100, Denis Laxalde wrote:
>> # HG changeset patch
>> # User Denis Laxalde <denis.laxalde at logilab.fr>
>> # Date 1476279051 -7200
>> #      Wed Oct 12 15:30:51 2016 +0200
>> # Node ID 0cf70234a38e47a3f7107611885368db9d52f574
>> # Parent  342d0cb4f446826169a83a6e773f1c767e0c977b
>> # EXP-Topic linerange-log/revset
>> mdiff: add a "blocksinrange" function to filter diff blocks by line range
>
> I haven't read this patch carefully, so I might misunderstand the algorithm.
>
>> diff --git a/mercurial/mdiff.py b/mercurial/mdiff.py
>> --- a/mercurial/mdiff.py
>> +++ b/mercurial/mdiff.py
>> @@ -113,6 +113,45 @@ def splitblock(base1, lines1, base2, lin
>>          s1 = i1
>>          s2 = i2
>>
>> +def blocksinrange(blocks, rangeb):
>> +    """yield ``block = (a1, a2, b1, b2), stype`` items of `blocks` that are
>> +    inside `rangeb` from ``(b1, b2)`` point of view; a block ``(b1, b2)``
>> +    being inside `rangeb` if ``rangeb[0] < b2 and b1 < rangeb[1]``.
>> +
>> +    Compute `rangea` w.r.t. to ``(a1, a2)`` parts of `blocks`, and bind it to
>> +    the final StopIteration exception.
>> +    """
>> +    lbb, ubb = rangeb
>> +    lba, uba = None, None
>> +    for block in blocks:
>> +        (a1, a2, b1, b2), stype = block
>> +        if lbb >= b1 and ubb <= b2 and stype == '=':
>> +            # rangeb is within a single "=" hunk, restrict back linerange1
>> +            # by offsetting rangeb
>> +            lba = lbb - b1 + a1
>> +            uba = ubb - b1 + a1
>> +        else:
>
>> +            if lbb == ubb and b1 <= ubb < b2:
>> +                # oneline range, within (b1, b2) block
>> +                lba = a1
>> +                uba = a2
>
> I'm not sure if this special case is necessary and valid because the condition
> "b1 <= ubb (== lbb) < b2" is slightly different from the other common cases.
> I don't get why lbb == ubb is special.

According to the tests I introduced, this special case is useless.
(Probably a leftover of algorigthm development.)

>> +            else:
>> +                if b1 <= lbb < b2:
>> +                    if stype == '=':
>> +                        lba = a2 - (b2 - lbb)
>> +                    else:
>> +                        lba = a1
>> +                if b1 < ubb <= b2:
>> +                    if stype == '=':
>> +                        uba = a1 + (ubb - b1)
>> +                    else:
>> +                        uba = a2
>> +        if lbb < b2 and b1 < ubb:
>> +            yield block
>
> Style nit: I prefer elif over deeply nested if-else.
>
>> +    if lba is None or uba is None or uba < lba:
>> +        raise ValueError('out of range')
>> +    raise StopIteration((lba, uba))
>
> Is it common to carry values by StopIteration? If not, maybe this could be a
> plain function which returns (blocks_in_rangeb, (lba, uba)).
>

This is what happens in Python 3 (PEP 380) when a generator returns a value.

I originally introduced this mechanism while attempting to propagate
this information up to patch.diff() (through mdiff.unidiff(), etc.) to
avoid making it a plain function (not a generator). Clearly this is not
useful in the scope of this patch set because blocks are always
consumed. So, since the other part of my work is not ready yet, I can
certainly avoid this pattern for now and eventually (re-)introduce it
later when it'd be needed.


More information about the Mercurial-devel mailing list