Patchwork [2,of,3,V2] revset: cleanup of algorithm for `destination()` predicate

login
register
mail settings
Submitter Hannes Oldenburg
Date Sept. 11, 2016, 3:05 p.m.
Message ID <05b4f301b5ade476f7f2.1473606359@localhost.localdomain>
Download mbox | patch
Permalink /patch/16585/
State Accepted
Headers show

Comments

Hannes Oldenburg - Sept. 11, 2016, 3:05 p.m.
# HG changeset patch
# User Hannes Oldenburg <hannes.christian.oldenburg@gmail.com>
# Date 1473454657 0
#      Fri Sep 09 20:57:37 2016 +0000
# Node ID 05b4f301b5ade476f7f2236f7bf699a4f248297d
# Parent  aeb4ff2efd119ad5bab99f0c521ca52f71144309
revset: cleanup of algorithm for `destination()` predicate

Instead of using break to leave the inner while loop if we found a destination
or a path to an already known destination, we make the check part of the
looping condition.
Yuya Nishihara - Sept. 12, 2016, 12:58 p.m.
On Sun, 11 Sep 2016 15:05:59 +0000, Hannes Oldenburg wrote:
> # HG changeset patch
> # User Hannes Oldenburg <hannes.christian.oldenburg@gmail.com>
> # Date 1473454657 0
> #      Fri Sep 09 20:57:37 2016 +0000
> # Node ID 05b4f301b5ade476f7f2236f7bf699a4f248297d
> # Parent  aeb4ff2efd119ad5bab99f0c521ca52f71144309
> revset: cleanup of algorithm for `destination()` predicate
> 
> Instead of using break to leave the inner while loop if we found a destination
> or a path to an already known destination, we make the check part of the
> looping condition.
> 
> diff -r aeb4ff2efd11 -r 05b4f301b5ad mercurial/revset.py
> --- a/mercurial/revset.py	Fri Sep 09 20:48:18 2016 +0000
> +++ b/mercurial/revset.py	Fri Sep 09 20:57:37 2016 +0000
> @@ -853,8 +853,8 @@
>      for r in subset:
>          src = _getrevsource(repo, r)
>          lineage = list()
> -
> -        while src is not None:
> +        found = False
> +        while src is not None and not found:
>              lineage.append(r)
>  
>              # The visited lineage is a match if the current source is in the arg
> @@ -863,13 +863,11 @@
>              # different iteration over subset.  Likewise, if the src was already
>              # selected, the current lineage can be selected without going back
>              # further.
> -            if src in sources or src in dests:
> -                dests.update(lineage)
> -                break
> -
> +            found = src in sources or src in dests

I don't think new code is cleaner. Early break sounds more natural than
using complicated loop condition.

If you prefer Pythonic for-loop, maybe you can use iter(callable, sentinel)
instead. See 0806fa2a39d8 for example.

> +        if found:
> +            dests.update(linage)

s/linage/lineage/

Patch

diff -r aeb4ff2efd11 -r 05b4f301b5ad mercurial/revset.py
--- a/mercurial/revset.py	Fri Sep 09 20:48:18 2016 +0000
+++ b/mercurial/revset.py	Fri Sep 09 20:57:37 2016 +0000
@@ -853,8 +853,8 @@ 
     for r in subset:
         src = _getrevsource(repo, r)
         lineage = list()
-
-        while src is not None:
+        found = False
+        while src is not None and not found:
             lineage.append(r)
 
             # The visited lineage is a match if the current source is in the arg
@@ -863,13 +863,11 @@ 
             # different iteration over subset.  Likewise, if the src was already
             # selected, the current lineage can be selected without going back
             # further.
-            if src in sources or src in dests:
-                dests.update(lineage)
-                break
-
+            found = src in sources or src in dests
             r = src
             src = _getrevsource(repo, r)
-
+        if found:
+            dests.update(linage)
     return subset.filter(dests.__contains__,
                          condrepr=lambda: '<destination %r>' % sorted(dests))