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

[Yuya Nishihara]

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.

> +            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)).