[PATCH STABLE] hgweb: add a `web.view` to control filtering

[Kevin Bullock]

On Jan 31, 2013, at 12:57 PM, pierre-yves.david at logilab.fr wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at logilab.fr>
> # Date 1359658615 -3600
> # Branch stable
> # Node ID 4a841ef7813f99bc7f091110e2f0dc8db82dfb86
> # Parent  2a1fac3650a5b4d650198604c82ab59969500374
> hgweb: add a `web.view` to control filtering
> 
> This options add a new `web.view` to control filter level of hgweb.
> 
> This option have two purposes:
> 
> 1) Allow fall back to unfiltered version in case a yet undetected by critical
>   bug is found in filtering after 2.5 release
> 
> 2) People use hgweb as a local repoviewer. When they have secret changesets,
>   they wants to use "visible" filter not "served"

As far as I can glean, the argument for this patch going in before tomorrow is:

2.5 introduces filtering: hgweb now hides secret changesets. This causes a known regression for one use case, and maybe possibly some other as-yet-unknown regressions. So we should provide an option to turn it off.

I don't buy this argument as regards (1) (unknown regressions); I might buy it as regards (2) (known regression).

> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1457,5 +1457,11 @@ The full set of options is:
> ``style``
>     Which template map style to use.
> 
> ``templates``
>     Where to find the HTML templates. Default is install path.
> +
> +``view``
> +    Controls Changesets filter to hgweb. Possible values are ``served``,
> +    ``visible`` and ``all``. Default is ``served``. The ``served`` filter only
> +    shows changesets that can be pulled from the hgweb instance. The``visible``
> +    filter includes secret changesets but still excludes "hidden" one.
> 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
> @@ -5,11 +5,11 @@
> #
> # This software may be used and distributed according to the terms of the
> # GNU General Public License version 2 or any later version.
> 
> import os
> -from mercurial import ui, hg, hook, error, encoding, templater, util
> +from mercurial import ui, hg, hook, error, encoding, templater, util, repoview
> 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
> from request import wsgirequest
> import webcommands, protocol, webutil
> @@ -57,11 +57,18 @@ class hgweb(object):
>                 u = ui.ui()
>             self.repo = hg.repository(u, repo)
>         else:
>             self.repo = repo
> 
> -        self.repo =  self.repo.filtered('served')
> +        viewconfig = self.config('web', 'view', 'served')
> +        if viewconfig == 'all':
> +            self.repo = self.repo.unfiltered()
> +        elif viewconfig in repoview.filtertable:
> +            self.repo = self.repo.filtered(viewconfig)
> +        else:
> +            self.repo = self.repo.filtered('served')
> +
>         self.repo.ui.setconfig('ui', 'report_untrusted', 'off')
>         self.repo.ui.setconfig('ui', 'nontty', 'true')
>         hook.redirect(True)
>         self.mtime = -1
>         self.size = -1
> @@ -94,11 +101,17 @@ class hgweb(object):
>         # rollbacks made less than a second ago
>         if st.st_mtime != self.mtime or st.st_size != self.size:
>             self.mtime = st.st_mtime
>             self.size = st.st_size
>             self.repo = hg.repository(self.repo.ui, self.repo.root)
> -            self.repo =  self.repo.filtered('served')
> +            viewconfig = self.config('web', 'view', 'served')
> +            if viewconfig == 'all':
> +                self.repo = self.repo.unfiltered()
> +            elif viewconfig in repoview.filtertable:
> +                self.repo = self.repo.filtered(viewconfig)
> +            else:
> +                self.repo = self.repo.filtered('served')

1. Why do we have to do this twice?
2. If we have to do it twice, why on earth wouldn't you wrap it in a function?

Before bringing up a code-churn argument, observe that you would be adding less lines of code (and half as much opportunity for a logic error!) if you added a new function.

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock