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

[Pierre-Yves David]

On 31 janv. 2013, at 20:10, Kevin Bullock wrote:

> 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?

Because there is two place where repo are created

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

Simpler patch for stable. I plan a follow up changeset for default that includes the (conditional) hg.repository call and the filtering switch.

> 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.

The diff is bigger but simpler.

-- 
Pierre-Yves David