Patchwork [08,of,11] py3: pass ctx.rev() instead of ctx in range()

login
register
mail settings
Submitter Pulkit Goyal
Date Feb. 23, 2018, 6:15 p.m.
Message ID <a2b469c5995b4c2602f1.1519409721@workspace>
Download mbox | patch
Permalink /patch/28290/
State Accepted
Headers show

Comments

Pulkit Goyal - Feb. 23, 2018, 6:15 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1519389273 -19800
#      Fri Feb 23 18:04:33 2018 +0530
# Node ID a2b469c5995b4c2602f1bb1c3959d67ee9c08141
# Parent  457aea9470d03f8e69c873b9aec845256abf9023
py3: pass ctx.rev() instead of ctx in range()
Gregory Szorc - Feb. 24, 2018, 2:54 a.m.
On Fri, Feb 23, 2018 at 10:15 AM, Pulkit Goyal <7895pulkit@gmail.com> wrote:

> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1519389273 -19800
> #      Fri Feb 23 18:04:33 2018 +0530
> # Node ID a2b469c5995b4c2602f1bb1c3959d67ee9c08141
> # Parent  457aea9470d03f8e69c873b9aec845256abf9023
> py3: pass ctx.rev() instead of ctx in range()
>
> diff --git a/hgext/acl.py b/hgext/acl.py
> --- a/hgext/acl.py
> +++ b/hgext/acl.py
> @@ -356,7 +356,7 @@ def hook(ui, repo, hooktype, node=None,
>      allow = buildmatch(ui, repo, user, 'acl.allow')
>      deny = buildmatch(ui, repo, user, 'acl.deny')
>
> -    for rev in xrange(repo[node], len(repo)):
> +    for rev in xrange(repo[node].rev(), len(repo)):
>

If someone could enlighten me as to why this works in Python 2 but not
Python 3, I would appreciate it. The new code is obviously correct. I'm
just curious what Python 3 changed about the semantics of range(). I guess
it no longer calls __int__???

Speaking of __int__, it is only defined in one place in the repo: on
context.basectx. I think we should consider removing it: it is too much
coercion magic for my liking.


>          ctx = repo[rev]
>          branch = ctx.branch()
>          if denybranches and denybranches(branch):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - Feb. 24, 2018, 5:08 a.m.
On Fri, 23 Feb 2018 18:54:53 -0800, Gregory Szorc wrote:
> Speaking of __int__, it is only defined in one place in the repo: on
> context.basectx. I think we should consider removing it: it is too much
> coercion magic for my liking.

+1 for removing int(ctx) and '%d' % ctx. Actually it doesn't always return
an int because rev() may be None.

Patch

diff --git a/hgext/acl.py b/hgext/acl.py
--- a/hgext/acl.py
+++ b/hgext/acl.py
@@ -356,7 +356,7 @@  def hook(ui, repo, hooktype, node=None, 
     allow = buildmatch(ui, repo, user, 'acl.allow')
     deny = buildmatch(ui, repo, user, 'acl.deny')
 
-    for rev in xrange(repo[node], len(repo)):
+    for rev in xrange(repo[node].rev(), len(repo)):
         ctx = repo[rev]
         branch = ctx.branch()
         if denybranches and denybranches(branch):