Patchwork [7,of,7] hgweb: convert _siblings to a factory function of mappinggenerator

login
register
mail settings
Submitter Yuya Nishihara
Date April 14, 2018, 12:49 p.m.
Message ID <fe959b32685068231cad.1523710157@mimosa>
Download mbox | patch
Permalink /patch/31027/
State Accepted
Headers show

Comments

Yuya Nishihara - April 14, 2018, 12:49 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1522594234 -32400
#      Sun Apr 01 23:50:34 2018 +0900
# Node ID fe959b32685068231cad8ef26387c7c16fe0961a
# Parent  8e479b1d96bf94e81f76c78605c16b2864b219a5
hgweb: convert _siblings to a factory function of mappinggenerator

IIUC, only reason it was a class is to make the generator restartable,
which is now served by the mappinggenerator.
Anton Shestakov - April 15, 2018, 5:54 a.m.
On Sat, 14 Apr 2018 21:49:17 +0900
Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1522594234 -32400
> #      Sun Apr 01 23:50:34 2018 +0900
> # Node ID fe959b32685068231cad8ef26387c7c16fe0961a
> # Parent  8e479b1d96bf94e81f76c78605c16b2864b219a5
> hgweb: convert _siblings to a factory function of mappinggenerator

Queued, thanks!
Anton Shestakov - April 15, 2018, 6:06 a.m.
On Sat, 14 Apr 2018 21:49:17 +0900
Yuya Nishihara <yuya@tcha.org> wrote:

> -class _siblings(object):
> -    def __init__(self, siblings=None, hiderev=None):
> -        if siblings is None:
> -            siblings = []
> -        self.siblings = [s for s in siblings if s.node() != nullid]
> -        if len(self.siblings) == 1 and self.siblings[0].rev() == hiderev:
> -            self.siblings = []
> -
> -    def __iter__(self):
> -        return _ctxsgen(self.siblings)
> -
> -    def __len__(self):
> -        return len(self.siblings)
> +def _siblings(siblings=None, hiderev=None):
> +    if siblings is None:
> +        siblings = []
> +    siblings = [s for s in siblings if s.node() != nullid]
> +    if len(siblings) == 1 and siblings[0].rev() == hiderev:
> +        siblings = []
> +    return templateutil.mappinggenerator(_ctxsgen, args=(siblings,))

One thing that this patch makes difficult to understand is how you can't
now say len(parents):

  TypeError: object of type 'mappinggenerator' has no len()

... but "{count(parents)}" in the templates works as expected, even
though count template filter simply does len(i).
Yuya Nishihara - April 15, 2018, 1:20 p.m.
On Sun, 15 Apr 2018 14:06:03 +0800, Anton Shestakov wrote:
> On Sat, 14 Apr 2018 21:49:17 +0900
> Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > -class _siblings(object):
> > -    def __init__(self, siblings=None, hiderev=None):
> > -        if siblings is None:
> > -            siblings = []
> > -        self.siblings = [s for s in siblings if s.node() != nullid]
> > -        if len(self.siblings) == 1 and self.siblings[0].rev() == hiderev:
> > -            self.siblings = []
> > -
> > -    def __iter__(self):
> > -        return _ctxsgen(self.siblings)
> > -
> > -    def __len__(self):
> > -        return len(self.siblings)
> > +def _siblings(siblings=None, hiderev=None):
> > +    if siblings is None:
> > +        siblings = []
> > +    siblings = [s for s in siblings if s.node() != nullid]
> > +    if len(siblings) == 1 and siblings[0].rev() == hiderev:
> > +        siblings = []
> > +    return templateutil.mappinggenerator(_ctxsgen, args=(siblings,))
> 
> One thing that this patch makes difficult to understand is how you can't
> now say len(parents):
> 
>   TypeError: object of type 'mappinggenerator' has no len()
> 
> ... but "{count(parents)}" in the templates works as expected, even
> though count template filter simply does len(i).

Filter functions receive an inner value, in which case, it is a list of dicts.

Basically I threw away the idea behind the hybrid class, which is to provide
all required behaviors by a single object as if it were a plain Python object.
It didn't work. Instead, a wrapped class can be considered a container like
Maybe type. It returns some meaningful values depending on what is expected by
caller.

Patch

diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
--- a/mercurial/hgweb/webutil.py
+++ b/mercurial/hgweb/webutil.py
@@ -183,7 +183,7 @@  class filerevnav(revnav):
 
 # TODO: maybe this can be a wrapper class for changectx/filectx list, which
 # yields {'ctx': ctx}
-def _ctxsgen(ctxs):
+def _ctxsgen(context, ctxs):
     for s in ctxs:
         d = {
             'node': s.hex(),
@@ -197,19 +197,13 @@  def _ctxsgen(ctxs):
             d['file'] = s.path()
         yield d
 
-class _siblings(object):
-    def __init__(self, siblings=None, hiderev=None):
-        if siblings is None:
-            siblings = []
-        self.siblings = [s for s in siblings if s.node() != nullid]
-        if len(self.siblings) == 1 and self.siblings[0].rev() == hiderev:
-            self.siblings = []
-
-    def __iter__(self):
-        return _ctxsgen(self.siblings)
-
-    def __len__(self):
-        return len(self.siblings)
+def _siblings(siblings=None, hiderev=None):
+    if siblings is None:
+        siblings = []
+    siblings = [s for s in siblings if s.node() != nullid]
+    if len(siblings) == 1 and siblings[0].rev() == hiderev:
+        siblings = []
+    return templateutil.mappinggenerator(_ctxsgen, args=(siblings,))
 
 def difffeatureopts(req, ui, section):
     diffopts = patch.difffeatureopts(ui, untrusted=True,