Patchwork dagop: make stopdepth in _genrevancestors() match revset arg

login
register
mail settings
Submitter via Mercurial-devel
Date June 23, 2017, 6:39 p.m.
Message ID <ba913ce1b38584554a62.1498243177@martinvonz.svl.corp.google.com>
Download mbox | patch
Permalink /patch/21643/
State Rejected
Headers show

Comments

via Mercurial-devel - June 23, 2017, 6:39 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1498241757 25200
#      Fri Jun 23 11:15:57 2017 -0700
# Node ID ba913ce1b38584554a62ba3d765d124c284e7e0d
# Parent  f63d111258dab8375322483888a44bf9be6c4673
dagop: make stopdepth in _genrevancestors() match revset arg

I was confused for a while that stopdepth was exclusive in dagop.py
but not in the ancestors() revset.

Let's also raise an exception if stopdepth<0, since revset.py already
checks that.
Yuya Nishihara - June 24, 2017, 3:23 a.m.
On Fri, 23 Jun 2017 11:39:37 -0700, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1498241757 25200
> #      Fri Jun 23 11:15:57 2017 -0700
> # Node ID ba913ce1b38584554a62ba3d765d124c284e7e0d
> # Parent  f63d111258dab8375322483888a44bf9be6c4673
> dagop: make stopdepth in _genrevancestors() match revset arg
> 
> I was confused for a while that stopdepth was exclusive in dagop.py
> but not in the ancestors() revset.

We generally use half-open range for internal stuff (e.g. revlog.revs(),
smartset.spanset), so I made stopdepth an exclusive boundary.

> Let's also raise an exception if stopdepth<0, since revset.py already
> checks that.

This seems good.
via Mercurial-devel - June 24, 2017, 4:47 a.m.
On Fri, Jun 23, 2017 at 8:23 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Fri, 23 Jun 2017 11:39:37 -0700, Martin von Zweigbergk wrote:
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz@google.com>
>> # Date 1498241757 25200
>> #      Fri Jun 23 11:15:57 2017 -0700
>> # Node ID ba913ce1b38584554a62ba3d765d124c284e7e0d
>> # Parent  f63d111258dab8375322483888a44bf9be6c4673
>> dagop: make stopdepth in _genrevancestors() match revset arg
>>
>> I was confused for a while that stopdepth was exclusive in dagop.py
>> but not in the ancestors() revset.
>
> We generally use half-open range for internal stuff (e.g. revlog.revs(),
> smartset.spanset), so I made stopdepth an exclusive boundary.

Fair enough.

>
>> Let's also raise an exception if stopdepth<0, since revset.py already
>> checks that.
>
> This seems good.

I'll send a patch for only that part.

Patch

diff --git a/mercurial/dagop.py b/mercurial/dagop.py
--- a/mercurial/dagop.py
+++ b/mercurial/dagop.py
@@ -32,8 +32,8 @@ 
         startdepth = 0
     if stopdepth is None:
         stopdepth = _maxlogdepth
-    if stopdepth <= 0:
-        return
+    if stopdepth < 0:
+        raise error.ProgrammingError('negative stopdepth')
 
     cl = repo.changelog
 
@@ -62,7 +62,7 @@ 
             lastrev = currev
             yield currev
         pdepth = curdepth + 1
-        if foundnew and pdepth < stopdepth:
+        if foundnew and pdepth <= stopdepth:
             try:
                 for prev in cl.parentrevs(currev)[:cut]:
                     if prev != node.nullrev:
@@ -76,7 +76,7 @@ 
     """Like revlog.ancestors(), but supports additional options, includes
     the given revs themselves, and returns a smartset
 
-    Scan ends at the stopdepth (exlusive) if specified. Revisions found
+    Scan ends at the stopdepth (inclusive) if specified. Revisions found
     earlier than the startdepth are omitted.
     """
     gen = _genrevancestors(repo, revs, followfirst, startdepth, stopdepth)
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -271,7 +271,7 @@ 
         n = getinteger(args['depth'], _("ancestors expects an integer depth"))
         if n < 0:
             raise error.ParseError(_("negative depth"))
-        stopdepth = n + 1
+        stopdepth = n
     return _ancestors(repo, subset, args['set'],
                       startdepth=startdepth, stopdepth=stopdepth)