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

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2815

hgweb: extract entries() to standalone function
ClosedPublic

Authored by indygreg on Mar 12 2018, 5:16 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG455918512ed2: hgweb: extract entries() to standalone function
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.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Mar 12 2018, 5:16 PM
durin42 accepted this revision.Mar 12 2018, 5:25 PM
This revision is now accepted and ready to land.Mar 12 2018, 5:25 PM
Closed by commit rHG455918512ed2: hgweb: extract entries() to standalone function (authored by indygreg). · Explain WhyMar 12 2018, 5:38 PM
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Mar 17 2018, 11:41 AM

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.

yuja added a comment.Mar 19 2018, 10:01 AM

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.

Revision Contents
Changeset List

PathPackages
Mmercurial/hgweb/hgwebdir_mod.py (35 lines)

Diff 6881

mercurial/hgweb/hgwebdir_mod.py

'lastchange_sort': d[1] - d[0], 'lastchange_sort': d[1] - d[0],
'archives': archivelist(u, "tip", url), 'archives': archivelist(u, "tip", url),
'isdirectory': None, 'isdirectory': None,
'labels': u.configlist('web', 'labels', untrusted=True), 'labels': u.configlist('web', 'labels', untrusted=True),
} }
yield row 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): class hgwebdir(object):
"""HTTP server for multiple repositories. """HTTP server for multiple repositories.
Given a configuration, different repositories will be served depending Given a configuration, different repositories will be served depending
on the request path. on the request path.
Instances are typically used as WSGI applications. Instances are typically used as WSGI applications.
""" """
wsgireq.respond(err, ctype) wsgireq.respond(err, ctype)
return tmpl('error', error=err.message or '') return tmpl('error', error=err.message or '')
finally: finally:
tmpl = None tmpl = None
def makeindex(self, wsgireq, tmpl, subdir=""): def makeindex(self, wsgireq, tmpl, subdir=""):
req = wsgireq.req 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() self.refresh()
sortable = ["name", "description", "contact", "lastchange"] sortable = ["name", "description", "contact", "lastchange"]
sortcolumn, descending = sortdefault sortcolumn, descending = None, False
if 'sort' in req.qsparams: if 'sort' in req.qsparams:
sortcolumn = req.qsparams['sort'] sortcolumn = req.qsparams['sort']
descending = sortcolumn.startswith('-') descending = sortcolumn.startswith('-')
if descending: if descending:
sortcolumn = sortcolumn[1:] sortcolumn = sortcolumn[1:]
if sortcolumn not in sortable: if sortcolumn not in sortable:
sortcolumn = "" sortcolumn = ""
sort = [("sort_%s" % column, sort = [("sort_%s" % column,
"%s%s" % ((not descending and column == sortcolumn) "%s%s" % ((not descending and column == sortcolumn)
and "-" or "", column)) and "-" or "", column))
for column in sortable] for column in sortable]
self.refresh() self.refresh()
self.updatereqenv(wsgireq.env) 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, return tmpl("index", entries=entries, subdir=subdir,
pathdef=hgweb_mod.makebreadcrumb('/' + subdir, self.prefix), pathdef=hgweb_mod.makebreadcrumb('/' + subdir, self.prefix),
sortcolumn=sortcolumn, descending=descending, sortcolumn=sortcolumn, descending=descending,
**dict(sort)) **dict(sort))
def templater(self, wsgireq, nonce): def templater(self, wsgireq, nonce):
def motd(**map): def motd(**map):