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

```Yuya Nishihara a écrit :
```
```On Mon, 28 Nov 2016 10:54:14 +0100, Denis Laxalde wrote:
```
```# HG changeset patch
# User Denis Laxalde <denis.laxa...@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.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
```