Patchwork D2815: hgweb: extract entries() to standalone function

login
register
mail settings
Submitter phabricator
Date March 12, 2018, 9:16 p.m.
Message ID <differential-rev-PHID-DREV-qhw4cmlbba45huh7wfkt-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/29325/
State Superseded
Headers show

Comments

phabricator - March 12, 2018, 9:16 p.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  There was some real wonkiness going on here. Essentially, the
  inline function was being executed with default arguments because
  a function reference was passed as-is into the templater. That
  seemed odd. So now we pass an explicit generator of the function
  result.
  
  Moving this code out of makeindex() makes makeindex() small enough
  to reason about. This makes it easier to see weird things, like the
  fact that we're calling self.refresh() twice. Why, I'm not sure.
  I'm also not sure why we need to call updatereqenv() to possibly
  update the SERVER_NAME, SERVER_PORT, and SCRIPT_NAME variables as
  part of rendering an index. I'll dig into these things in subsequent
  commits.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2815

AFFECTED FILES
  mercurial/hgweb/hgwebdir_mod.py

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 17, 2018, 3:41 p.m.
yuja added a comment.


  > Essentially, the inline function was being executed with default arguments
  >  because a function reference was passed as-is into the templater. That
  >  seemed odd. So now we pass an explicit generator of the function
  >  result.
  
  This move is wrong because a template keyword may be evaluated more
  than once. If it's a generator, the first `{entries}` consumes the entire data
   and the second `{entries}` would be empty.
  
  Can you send a follow up? `entries` can be a list or a function returning
  a generator.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2815

To: indygreg, #hg-reviewers, durin42
Cc: yuja, mercurial-devel
phabricator - March 19, 2018, 2:01 p.m.
yuja added a comment.


  > `entries` can be a list or a function returning a generator.
  
  I'm going to add a wrapper class for a generator of mappings, so this can be
  addressed later.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2815

To: indygreg, #hg-reviewers, durin42
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/hgweb/hgwebdir_mod.py b/mercurial/hgweb/hgwebdir_mod.py
--- a/mercurial/hgweb/hgwebdir_mod.py
+++ b/mercurial/hgweb/hgwebdir_mod.py
@@ -273,6 +273,22 @@ 
 
         yield row
 
+def indexentries(ui, repos, wsgireq, req, stripecount, sortcolumn='',
+                 descending=False, subdir='', **map):
+
+    rows = rawindexentries(ui, repos, wsgireq, req, subdir=subdir, **map)
+
+    sortdefault = None, False
+
+    if sortcolumn and sortdefault != (sortcolumn, descending):
+        sortkey = '%s_sort' % sortcolumn
+        rows = sorted(rows, key=lambda x: x[sortkey],
+                      reverse=descending)
+
+    for row, parity in zip(rows, paritygen(stripecount)):
+        row['parity'] = parity
+        yield row
+
 class hgwebdir(object):
     """HTTP server for multiple repositories.
 
@@ -472,22 +488,9 @@ 
     def makeindex(self, wsgireq, tmpl, subdir=""):
         req = wsgireq.req
 
-        sortdefault = None, False
-        def entries(sortcolumn="", descending=False, subdir="", **map):
-            rows = rawindexentries(self.ui, self.repos, wsgireq, req,
-                                   subdir=subdir, **map)
-
-            if sortcolumn and sortdefault != (sortcolumn, descending):
-                sortkey = '%s_sort' % sortcolumn
-                rows = sorted(rows, key=lambda x: x[sortkey],
-                              reverse=descending)
-            for row, parity in zip(rows, paritygen(self.stripecount)):
-                row['parity'] = parity
-                yield row
-
         self.refresh()
         sortable = ["name", "description", "contact", "lastchange"]
-        sortcolumn, descending = sortdefault
+        sortcolumn, descending = None, False
         if 'sort' in req.qsparams:
             sortcolumn = req.qsparams['sort']
             descending = sortcolumn.startswith('-')
@@ -504,6 +507,10 @@ 
         self.refresh()
         self.updatereqenv(wsgireq.env)
 
+        entries = indexentries(self.ui, self.repos, wsgireq, req,
+                               self.stripecount, sortcolumn=sortcolumn,
+                               descending=descending, subdir=subdir)
+
         return tmpl("index", entries=entries, subdir=subdir,
                     pathdef=hgweb_mod.makebreadcrumb('/' + subdir, self.prefix),
                     sortcolumn=sortcolumn, descending=descending,