[PATCH 2 of 3] hgweb: move entry-preparing code from webcommands to webutils.makeentry()

[Anton Shestakov]

On Wed, 18 Nov 2015 23:27:18 +0900
Yuya Nishihara <yuya at tcha.org> wrote:

> On Tue, 17 Nov 2015 21:33:57 +0800, Anton Shestakov wrote:
> > # HG changeset patch
> > # User Anton Shestakov <av6 at dwimlabs.net>
> > # Date 1447396536 -28800
> > #      Fri Nov 13 14:35:36 2015 +0800
> > # Node ID 8fcd6c9323c1affd43a7ac5ee3b3c598b646bde7
> > # Parent  f6968049b048311f60d496f3c899e58fb649d884
> > hgweb: move entry-preparing code from webcommands to
> > webutils.makeentry()
> > 
> > The new function is used to prefill basic information about a ctx,
> > such as revision number and hash, author, commit message, etc.
> > Before, every webcommand used to get this basic information on its
> > own using some boilerplate code, and some things in some places
> > just weren't available (e.g. branch/tag/bookmark info for the
> > current hgweb revision in filelog).
> > 
> > diff --git a/mercurial/hgweb/webcommands.py
> > b/mercurial/hgweb/webcommands.py ---
> > a/mercurial/hgweb/webcommands.py +++
> > b/mercurial/hgweb/webcommands.py @@ -116,24 +116,14 @@ def
> > _filerevision(web, req, tmpl, fctx): "linenumber": "% 6d" % (lineno
> > + 1), "parity": parity.next()}
> >  
> > -    return tmpl("filerevision",
> > -                file=f,
> > -                path=webutil.up(f),
> > -                text=lines(),
> > -                rev=fctx.rev(),
> > -                symrev=webutil.symrevorshortnode(req, fctx),
> > -                node=fctx.hex(),
> > -                author=fctx.user(),
> > -                date=fctx.date(),
> > -                desc=fctx.description(),
> > -                extra=fctx.extra(),
> > -                branch=webutil.nodebranchnodefault(fctx),
> > -                parent=webutil.parents(fctx),
> > -                child=webutil.children(fctx),
> > -                rename=webutil.renamelink(fctx),
> > -                tags=webutil.nodetagsdict(web.repo, fctx.node()),
> > -                bookmarks=webutil.nodebookmarksdict(web.repo,
> > fctx.node()),
> > -                permissions=fctx.manifest().flags(f))
> > +    return tmpl("filerevision", **webutil.makeentry(web.repo,
> > fctx, {
> > +        'file': f,
> > +        'path': webutil.up(f),
> > +        'text': lines(),
> > +        'symrev': webutil.symrevorshortnode(req, fctx),
> > +        'rename': webutil.renamelink(fctx),
> > +        'permissions': fctx.manifest().flags(f),
> > +    }))  
> 
> This is minor thing, but I prefer
> 
>     return tmpl("filerevision",
>                 file=f,
>                 ...
>                 **webutil.commonentry(web.repo, fctx))
> 
> because webutil has similar functions named xxxentry(). A good thing
> of this style is that you can get TypeError if rev=ctx.rev() is
> specified twice, for example.

It might be a nice bonus, but makeentry() is supposed to first fill some
defaults and then allow to override them (that happens in
webutil.changelist() entry with "parent" and "child"). So sometimes
you're supposed to have a duplicate key. Also, because the result of
this function needs to have lower priority than the explicit keyword
args to tmpl(), and it's a syntax error to have something like this:

tmpl("filerevision", **commonentry(web.repo, fctx), file=f)

... the only other possible syntax I see is this:

tmpl("filerevision", **commonentry(web.repo, fctx, file=f))

Which, thanks to python's coding style, quickly devolves into:

tmpl("filerevision", **commonentry(web.repo, fctx,
                                   file=f,
                                   text=lines(),
                                   ...))

And that indentation level kills my soul. So I stick with {}, which
needs to be indented much less.

I'm open to different names for the function (after all, naming is
hard), but if I saw a function called commonentry(), I'd think it's
what there's in makeentry() now, but without entry.update(more). I.e.
commonentry() implies that it just fills common data and then you're
supposed to .update() it with extra things on your own. makeentry()
sounds like it could do something like that (and it does).

Anyway, I can resend fixed patch #1 and then, if you would expand you
yesterday's idea and/or give some hints, I could work on making a
separate templatekw table for hgweb instead. That sounds like a much
better idea, actually.