| Submitter | Pierre-Yves David |
|---|---|
| Date | Jan. 31, 2013, 6:57 p.m. |
| Message ID | <4a841ef7813f99bc7f09.1359658628@crater2.logilab.fr> |
| Download | mbox | patch |
| Permalink | /patch/774/ |
| State | Accepted |
| Commit | 36549fa712da69eabe4b4abbbedeb4546dc7d904 |
| Headers | show |
Comments
On Jan 31, 2013 7:57 PM, <pierre-yves.david@logilab.fr> wrote: > > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@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" > > 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. To my non native English speaker ears "served" seemed at first to imply revisions that _have been_ served, rather than they _can be_ served. This confused me at first since I thought this mode would not show revisions that have been pushed to the server. This is nonsense of course but perhaps another word could be used instead? Perhaps "accessible" or something similar? Also, if as you say users use hgweb as a way to view the history graphically, wouldn't it be necessary to also add a way to switch modes right from the hgweb interface (this option would set the default view)? (And to be done after the freeze). Cheers, Angel
On 31 janv. 2013, at 20:26, Angel Ezquerra wrote: > > On Jan 31, 2013 7:57 PM, <pierre-yves.david@logilab.fr> wrote: > > > > # HG changeset patch > > # User Pierre-Yves David <pierre-yves.david@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" > > > > 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. > > To my non native English speaker ears "served" seemed at first to imply revisions that _have been_ served, rather than they _can be_ served. This confused me at first since I thought this mode would not show revisions that have been pushed to the server. This is nonsense of course but perhaps another word could be used instead? Perhaps "accessible" or something similar? For my part I read "served" as "being served". As "served" is the internal name of the filter used everywhere in the code, this is unlikely to changes. > Also, if as you say users use hgweb as a way to view the history graphically, wouldn't it be necessary to also add a way to switch modes right from the hgweb interface (this option would set the default view)? (And to be done after the freeze). Having a way to control the mode in the interface is something that'll probably want at some point. But this is a very low priority feature for me.
On 31 janv. 2013, at 20:10, Kevin Bullock wrote: > On Jan 31, 2013, at 12:57 PM, pierre-yves.david@logilab.fr wrote: > >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@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.
On Thu, 2013-01-31 at 19:57 +0100, pierre-yves.david@logilab.fr wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@logilab.fr> > # Date 1359658615 -3600 > # Branch stable > # Node ID 4a841ef7813f99bc7f091110e2f0dc8db82dfb86 > # Parent 2a1fac3650a5b4d650198604c82ab59969500374 > hgweb: add a `web.view` to control filtering I've queued a modified version of this using a helper function and without the docs. As I requested this primarily as insurance against regressions, I want to wait on promoting it to a first-class documented feature a little bit. Please resend the doc piece in a couple weeks.
Patch
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') self.maxchanges = int(self.config("web", "maxchanges", 10)) self.stripecount = int(self.config("web", "stripes", 1)) self.maxshortchanges = int(self.config("web", "maxshortchanges", 60)) self.maxfiles = int(self.config("web", "maxfiles", 10)) diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t --- a/tests/test-obsolete.t +++ b/tests/test-obsolete.t @@ -741,10 +741,28 @@ check graph view check filelog view $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'log/'`hg id --debug --id`/'babar' 200 Script output follows + + $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/68' + 200 Script output follows + $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/67' + 500 Internal Server Error + [1] + +check that web.view config option: + + $ kill `cat hg.pid` + $ cat >> .hg/hgrc << EOF + > [web] + > view=all + > EOF + $ wait + $ hg serve -n test -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log + $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/67' + 200 Script output follows $ kill `cat hg.pid` Checking _enable=False warning if obsolete marker exists $ echo '[extensions]' >> $HGRCPATH