[PATCH 1 of 3] hgweb, templates: add websub template filter

[Mads Kiilerich]

On 02/09/2013 11:02 AM, Angel Ezquerra wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra at gmail.com>
> # Date 1360343132 -3600
> # Node ID d2fe4130209f6877d841926df79ba02842c83e20
> # Parent  766ad3e48bdff8ee2b2a3a9276eff398dcaafa02
> hgweb, templates: add websub template filter
>
> The purpose of this new filter is to make it possible to partially replace the
> functionality of the interhg extension. The idea is to be able to define regular
> expression based substitutions on a new "websub" config section. hgweb will then
> be able to apply these substitutions wherever the "websub" filter is used on a
> template.
>
> This first revision just adds the code necessary to load the websub expressions
> and adds the websub filter, but it does not add any calls to the websub filter
> itself on any of the templates. That will be done on the following revisions.
>
> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
> --- a/mercurial/hgweb/hgweb_mod.py
> +++ b/mercurial/hgweb/hgweb_mod.py
> @@ -8,6 +8,7 @@
>   
>   import os
>   from mercurial import ui, hg, hook, error, encoding, templater, util, repoview
> +from mercurial import templatefilters
>   from common import get_stat, ErrorResponse, permhooks, caching
>   from common import HTTP_OK, HTTP_NOT_MODIFIED, HTTP_BAD_REQUEST
>   from common import HTTP_NOT_FOUND, HTTP_SERVER_ERROR
> @@ -49,6 +50,9 @@
>           urlel = os.path.dirname(urlel)
>       return reversed(breadcrumb)
>   
> +from mercurial.i18n import _
> +import re

Please keep the imports at the top and properly grouped.

> +websubtable = []
>   
>   class hgweb(object):
>       def __init__(self, repo, name=None, baseui=None):
> @@ -73,6 +77,7 @@
>           # a repo owner may set web.templates in .hg/hgrc to get any file
>           # readable by the user running the CGI script
>           self.templatepath = self.config('web', 'templates')
> +        self.websubtable = self.loadwebsub()
>   
>       # The CGI scripts are often run by a user different from the repo owner.
>       # Trust the settings from the .hg/hgrc files by default.
> @@ -258,6 +263,43 @@
>                   return ['']
>               return tmpl('error', error=inst.message)
>   
> +    def loadwebsub(self):
> +        websubtable = []
> +        for key, pattern in self.repo.ui.configitems('websub'):
> +            # grab the delimiter from the character after the "s"
> +            unesc = pattern[1]

I guess this will crash on single character patterns? Handling of this 
should be covered by some kind of test.

> +            delim = re.escape(unesc)
> +
> +            # identify portions of the pattern, taking care to avoid escaped
> +            # delimiters. the replace format and flags are optional, but delimiters
> +            # are required.
> +            match = re.match(r'^s%s(.+)(?:(?<=\\\\)|(?<!\\))%s(.*)%s([ilmsux])*$'
> +                             % (delim, delim, delim), pattern)

We usually don't rely on the internal re cache. Instead we prefer to 
compile regexps and store them in 'global' variables.

> +            if not match:
> +                self.repo.ui.warn(_("websub: invalid pattern for %s: %s\n")
> +                                  % (key, pattern))
> +                continue
> +
> +            # we need to unescape the delimiter for regexp and format
> +            delim_re = re.compile(r'(?<!\\)\\%s' % delim)
> +            regexp = delim_re.sub(unesc, match.group(1))
> +            format = delim_re.sub(unesc, match.group(2))
> +
> +            # the pattern allows for 6 regexp flags, so set them if necessary
> +            flagin = match.group(3)
> +            flags = 0
> +            if flagin:
> +                for flag in flagin.upper():
> +                    flags |= re.__dict__[flag]
> +
> +            try:
> +                regexp = re.compile(regexp, flags)
> +                websubtable.append((regexp, format))
> +            except re.error:
> +                self.repo.ui.warn(_("websub: invalid regexp for %s: %s\n")
> +                                  % (key, regexp))

The body of this loop looks like something that it would be nice to have 
in a unit tested util function.

> +        return websubtable
> +
>       def templater(self, req):
>   
>           # determine scheme, port and server name
> @@ -312,8 +354,8 @@
>                                or req.url.strip('/') or self.repo.root)
>   
>           # create the templater
> -
>           tmpl = templater.templater(mapfile,
> +                                   filters={"websub": lambda text: templatefilters.websub(text, self.websubtable)},

Did this really pass check-code.py?

>                                      defaults={"url": req.url,
>                                                "logourl": logourl,
>                                                "logoimg": logoimg,
> @@ -325,6 +367,7 @@
>                                                "motd": motd,
>                                                "sessionvars": sessionvars,
>                                                "pathdef": makebreadcrumb(req.url),
> +                                             "websubtable": self.websubtable,
>                                               })
>           return tmpl
>   
> diff --git a/mercurial/templatefilters.py b/mercurial/templatefilters.py
> --- a/mercurial/templatefilters.py
> +++ b/mercurial/templatefilters.py
> @@ -391,6 +391,15 @@
>       "xmlescape": xmlescape,
>   }
>   
> +def websub(text, websubtable):
> +    """:websub: Any text. Only applies to hgweb. Applies the regular
> +    expression replacements defined in the websub section.
> +    """
> +    if websubtable:
> +        for regexp, format in websubtable:
> +            text = regexp.sub(format, text)
> +    return text

Why do this filtering have to be a hgweb specific feature? Couldn't it 
be a generic templating filter?

> +
>   def fillfunc(context, mapping, args):
>       if not (1 <= len(args) <= 2):
>           raise error.ParseError(_("fill expects one or two arguments"))
> @@ -418,6 +427,7 @@
>   funcs = {
>       "fill": fillfunc,
>       "date": datefunc,
> +    #"linkify": linkify,

???

It seems like we don't have any tests for using custom styles in hgweb - 
tests that would ensure that we stayed backward compatible. That would 
have been the obvious place to test this ... Making it a generic 
templating feature would of course make it even simpler to test it ;-)

/Mads