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