Patchwork [1,of,3] webutil: make _siblings into an object with __iter__ and __len__

login
register
mail settings
Submitter Anton Shestakov
Date Nov. 17, 2015, 1:33 p.m.
Message ID <f6968049b048311f60d4.1447767236@neuro>
Download mbox | patch
Permalink /patch/11426/
State Superseded
Delegated to: Yuya Nishihara
Headers show

Comments

Anton Shestakov - Nov. 17, 2015, 1:33 p.m.
# HG changeset patch
# User Anton Shestakov <av6@dwimlabs.net>
# Date 1447147360 -28800
#      Tue Nov 10 17:22:40 2015 +0800
# Node ID f6968049b048311f60d496f3c899e58fb649d884
# Parent  2da6a2dbfc42bdec4bcaf47da947c64ff959a59c
webutil: make _siblings into an object with __iter__ and __len__

_siblings is a helper that is used for displaying changeset parents and
children in hgweb. Before, when it was a simple generator, it couldn't tell its
length without being consumed, and that required a special case when preparing
data for changeset template (see 9e1f4c65f5f5).

Let's make it into a class (similar to templatekw._hybrid) that allows len(...)
without side-effects.
Yuya Nishihara - Nov. 17, 2015, 3:11 p.m.
On Tue, 17 Nov 2015 21:33:56 +0800, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1447147360 -28800
> #      Tue Nov 10 17:22:40 2015 +0800
> # Node ID f6968049b048311f60d496f3c899e58fb649d884
> # Parent  2da6a2dbfc42bdec4bcaf47da947c64ff959a59c
> webutil: make _siblings into an object with __iter__ and __len__
> 
> _siblings is a helper that is used for displaying changeset parents and
> children in hgweb. Before, when it was a simple generator, it couldn't tell its
> length without being consumed, and that required a special case when preparing
> data for changeset template (see 9e1f4c65f5f5).
> 
> Let's make it into a class (similar to templatekw._hybrid) that allows len(...)
> without side-effects.
> 
> diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
> --- a/mercurial/hgweb/webutil.py
> +++ b/mercurial/hgweb/webutil.py
> @@ -124,20 +124,29 @@ class filerevnav(revnav):
>      def hex(self, rev):
>          return hex(self._changelog.node(self._revlog.linkrev(rev)))
>  
> +class _siblings(object):
> +    def __init__(self, siblings=[], hiderev=None):
> +        self.siblings = [s for s in siblings if s.node() != nullid]
> +        self.hiderev = hiderev
>  
> -def _siblings(siblings=[], hiderev=None):
> -    siblings = [s for s in siblings if s.node() != nullid]
> -    if len(siblings) == 1 and siblings[0].rev() == hiderev:
> -        return
> -    for s in siblings:
> -        d = {'node': s.hex(), 'rev': s.rev()}
> -        d['user'] = s.user()
> -        d['date'] = s.date()
> -        d['description'] = s.description()
> -        d['branch'] = s.branch()
> -        if util.safehasattr(s, 'path'):
> -            d['file'] = s.path()
> -        yield d
> +    def __iter__(self):
> +        if len(self) == 1 and self.siblings[0].rev() == self.hiderev:
> +            return
> +        for s in self.siblings:
> +            d = {
> +                'node': s.hex(),
> +                'rev': s.rev(),
> +                'user': s.user(),
> +                'date': s.date(),
> +                'description': s.description(),
> +                'branch': s.branch(),
> +            }
> +            if util.safehasattr(s, 'path'):
> +                d['file'] = s.path()
> +            yield d
> +
> +    def __len__(self):
> +        return len(self.siblings)
>  
>  def parents(ctx, hide=None):
>      if isinstance(ctx, context.basefilectx):
> @@ -354,7 +363,7 @@ def changesetentry(web, req, tmpl, ctx):
>          rev=ctx.rev(),
>          node=ctx.hex(),
>          symrev=symrevorshortnode(req, ctx),
> -        parent=tuple(parents(ctx)),
> +        parent=parents(ctx),
>          child=children(ctx),
>          basenode=basectx.hex(),
>          changesettag=showtags,

I'll look these patches deeply tomorrow.

That said, I think the hgweb should have its own templatekw table. As you know,
it's troublesome to put a generator into the mapping dict, and we have lambdas
(thunks) to evaluate slow keywords lazily, and there's no help about web
templates, etc. Those are what the templatekw module solves in command-line
layer.

Regards,
Yuya Nishihara - Nov. 18, 2015, 1:49 p.m.
On Tue, 17 Nov 2015 21:33:56 +0800, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1447147360 -28800
> #      Tue Nov 10 17:22:40 2015 +0800
> # Node ID f6968049b048311f60d496f3c899e58fb649d884
> # Parent  2da6a2dbfc42bdec4bcaf47da947c64ff959a59c
> webutil: make _siblings into an object with __iter__ and __len__
> 
> _siblings is a helper that is used for displaying changeset parents and
> children in hgweb. Before, when it was a simple generator, it couldn't tell its
> length without being consumed, and that required a special case when preparing
> data for changeset template (see 9e1f4c65f5f5).
> 
> Let's make it into a class (similar to templatekw._hybrid) that allows len(...)
> without side-effects.
> 
> diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
> --- a/mercurial/hgweb/webutil.py
> +++ b/mercurial/hgweb/webutil.py
> @@ -124,20 +124,29 @@ class filerevnav(revnav):
>      def hex(self, rev):
>          return hex(self._changelog.node(self._revlog.linkrev(rev)))
>  
> +class _siblings(object):
> +    def __init__(self, siblings=[], hiderev=None):
> +        self.siblings = [s for s in siblings if s.node() != nullid]
> +        self.hiderev = hiderev
>  
> -def _siblings(siblings=[], hiderev=None):
> -    siblings = [s for s in siblings if s.node() != nullid]
> -    if len(siblings) == 1 and siblings[0].rev() == hiderev:
> -        return
> -    for s in siblings:
> -        d = {'node': s.hex(), 'rev': s.rev()}
> -        d['user'] = s.user()
> -        d['date'] = s.date()
> -        d['description'] = s.description()
> -        d['branch'] = s.branch()
> -        if util.safehasattr(s, 'path'):
> -            d['file'] = s.path()
> -        yield d
> +    def __iter__(self):
> +        if len(self) == 1 and self.siblings[0].rev() == self.hiderev:
> +            return
> +        for s in self.siblings:
> +            d = {
> +                'node': s.hex(),
> +                'rev': s.rev(),
> +                'user': s.user(),
> +                'date': s.date(),
> +                'description': s.description(),
> +                'branch': s.branch(),
> +            }
> +            if util.safehasattr(s, 'path'):
> +                d['file'] = s.path()
> +            yield d
> +
> +    def __len__(self):
> +        return len(self.siblings)

If siblings[0] is hiderev, len() should be 0.
Anton Shestakov - Nov. 18, 2015, 2:45 p.m.
On Wed, 18 Nov 2015 22:49:56 +0900
Yuya Nishihara <yuya@tcha.org> wrote:

> On Tue, 17 Nov 2015 21:33:56 +0800, Anton Shestakov wrote:
> > # HG changeset patch
> > # User Anton Shestakov <av6@dwimlabs.net>
> > # Date 1447147360 -28800
> > #      Tue Nov 10 17:22:40 2015 +0800
> > # Node ID f6968049b048311f60d496f3c899e58fb649d884
> > # Parent  2da6a2dbfc42bdec4bcaf47da947c64ff959a59c
> > webutil: make _siblings into an object with __iter__ and __len__
> > 
> > _siblings is a helper that is used for displaying changeset parents
> > and children in hgweb. Before, when it was a simple generator, it
> > couldn't tell its length without being consumed, and that required
> > a special case when preparing data for changeset template (see
> > 9e1f4c65f5f5).
> > 
> > Let's make it into a class (similar to templatekw._hybrid) that
> > allows len(...) without side-effects.
> > 
> > diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
> > --- a/mercurial/hgweb/webutil.py
> > +++ b/mercurial/hgweb/webutil.py
> > @@ -124,20 +124,29 @@ class filerevnav(revnav):
> >      def hex(self, rev):
> >          return hex(self._changelog.node(self._revlog.linkrev(rev)))
> >  
> > +class _siblings(object):
> > +    def __init__(self, siblings=[], hiderev=None):
> > +        self.siblings = [s for s in siblings if s.node() != nullid]
> > +        self.hiderev = hiderev
> >  
> > -def _siblings(siblings=[], hiderev=None):
> > -    siblings = [s for s in siblings if s.node() != nullid]
> > -    if len(siblings) == 1 and siblings[0].rev() == hiderev:
> > -        return
> > -    for s in siblings:
> > -        d = {'node': s.hex(), 'rev': s.rev()}
> > -        d['user'] = s.user()
> > -        d['date'] = s.date()
> > -        d['description'] = s.description()
> > -        d['branch'] = s.branch()
> > -        if util.safehasattr(s, 'path'):
> > -            d['file'] = s.path()
> > -        yield d
> > +    def __iter__(self):
> > +        if len(self) == 1 and self.siblings[0].rev() ==
> > self.hiderev:
> > +            return
> > +        for s in self.siblings:
> > +            d = {
> > +                'node': s.hex(),
> > +                'rev': s.rev(),
> > +                'user': s.user(),
> > +                'date': s.date(),
> > +                'description': s.description(),
> > +                'branch': s.branch(),
> > +            }
> > +            if util.safehasattr(s, 'path'):
> > +                d['file'] = s.path()
> > +            yield d
> > +
> > +    def __len__(self):
> > +        return len(self.siblings)  
> 
> If siblings[0] is hiderev, len() should be 0.

Oh, you're right. I'll move that "if" from __iter__ to __init__.

Patch

diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
--- a/mercurial/hgweb/webutil.py
+++ b/mercurial/hgweb/webutil.py
@@ -124,20 +124,29 @@  class filerevnav(revnav):
     def hex(self, rev):
         return hex(self._changelog.node(self._revlog.linkrev(rev)))
 
+class _siblings(object):
+    def __init__(self, siblings=[], hiderev=None):
+        self.siblings = [s for s in siblings if s.node() != nullid]
+        self.hiderev = hiderev
 
-def _siblings(siblings=[], hiderev=None):
-    siblings = [s for s in siblings if s.node() != nullid]
-    if len(siblings) == 1 and siblings[0].rev() == hiderev:
-        return
-    for s in siblings:
-        d = {'node': s.hex(), 'rev': s.rev()}
-        d['user'] = s.user()
-        d['date'] = s.date()
-        d['description'] = s.description()
-        d['branch'] = s.branch()
-        if util.safehasattr(s, 'path'):
-            d['file'] = s.path()
-        yield d
+    def __iter__(self):
+        if len(self) == 1 and self.siblings[0].rev() == self.hiderev:
+            return
+        for s in self.siblings:
+            d = {
+                'node': s.hex(),
+                'rev': s.rev(),
+                'user': s.user(),
+                'date': s.date(),
+                'description': s.description(),
+                'branch': s.branch(),
+            }
+            if util.safehasattr(s, 'path'):
+                d['file'] = s.path()
+            yield d
+
+    def __len__(self):
+        return len(self.siblings)
 
 def parents(ctx, hide=None):
     if isinstance(ctx, context.basefilectx):
@@ -354,7 +363,7 @@  def changesetentry(web, req, tmpl, ctx):
         rev=ctx.rev(),
         node=ctx.hex(),
         symrev=symrevorshortnode(req, ctx),
-        parent=tuple(parents(ctx)),
+        parent=parents(ctx),
         child=children(ctx),
         basenode=basectx.hex(),
         changesettag=showtags,