| Submitter | via Mercurial-devel |
|---|---|
| Date | March 14, 2017, 7:03 a.m. |
| Message ID | <4b4b0fb20fc768c1ea6b.1489475033@martinvonz.mtv.corp.google.com> |
| Download | mbox | patch |
| Permalink | /patch/19330/ |
| State | Superseded |
| Headers | show |
Comments
On Tue, 14 Mar 2017 00:03:53 -0700, Martin von Zweigbergk via Mercurial-devel wrote: > # HG changeset patch > # User Martin von Zweigbergk <martinvonz@google.com> > # Date 1489466597 25200 > # Mon Mar 13 21:43:17 2017 -0700 > # Node ID 4b4b0fb20fc768c1ea6b330a5f37f3e325e88b32 > # Parent d17402c07698443c13b082df8a8635a82b83d097 > pager: if old pager extensions is enabled, respect pager.attend > > This patch makes us respect pager.attend again if the extension is > enabled. It also brings back the default attend list, so e.g. summary > is not paged by default, just like it used to be before pager was > moved into core. Does it mean we'll support the legacy pager extension forever? That might cause confusion as new users will be likely to see new pager behavior, but old users would be too lazy to update their hgrc.
On Mar 14, 2017 2:05 AM, "Yuya Nishihara" <yuya@tcha.org> wrote: On Tue, 14 Mar 2017 00:03:53 -0700, Martin von Zweigbergk via Mercurial-devel wrote: > # HG changeset patch > # User Martin von Zweigbergk <martinvonz@google.com> > # Date 1489466597 25200 > # Mon Mar 13 21:43:17 2017 -0700 > # Node ID 4b4b0fb20fc768c1ea6b330a5f37f3e325e88b32 > # Parent d17402c07698443c13b082df8a8635a82b83d097 > pager: if old pager extensions is enabled, respect pager.attend > > This patch makes us respect pager.attend again if the extension is > enabled. It also brings back the default attend list, so e.g. summary > is not paged by default, just like it used to be before pager was > moved into core. Does it mean we'll support the legacy pager extension forever? That might cause confusion as new users will be likely to see new pager behavior, but old users would be too lazy to update their hgrc. We have traditionally been very conservative about BC and I'm worried we're breaking it too much without this patch. We had a report internally about this breakage when we released a new hg version yesterday. We have a log alias called xl that we also have enabled paging for by default. Our user was setting pager.attend=<all our defaults, except xl> as a way of not getting xl paged. When we upgraded to the new hg, without disabling the extension, his "hg xl" output started getting paged. It also wasn't obvious how to get back the old behavior. He tried setting pager.attend-xl=no, but that only works for non-aliases. Setting it in pager.ignore also doesn't work until they have disabled the pager extension (and it requires it to be "log", not "xl"), and requiring two simultaneous changes makes it really tricky. Kyle and I spent around 20 minutes trying to figure out the right way of getting his behavior back to what he wanted. Also, it wasn't exactly the same as before, since all log aliases would now be treated the same. That last thing is usually desirable, so probably a good thing, but I do think the other pieces were too complicated for users. I'm not sure we need to support the legacy pager forever, but I do think we need to make the transition smoother. I'm much more concerned about changing behavior for existing users than the confusion about existing users and new users getting different behavior. Even if we do end up supporting the legacy pager forever, it's only about 50 lines of code, and it's pretty isolated in pager.py.
Patch
diff -r d17402c07698 -r 4b4b0fb20fc7 hgext/pager.py --- a/hgext/pager.py Mon Mar 13 21:42:59 2017 -0700 +++ b/hgext/pager.py Mon Mar 13 21:43:17 2017 -0700 @@ -64,10 +64,10 @@ # behavior is preserved. ui.setconfig('pager', 'ignore', '', 'pager') ui.pager('extension-via-attend-' + cmd) + else: + ui.disablepager() return orig(ui, options, cmd, cmdfunc) extensions.wrapfunction(dispatch, '_runcommand', pagecmd) -attended = [ - 'the-default-attend-list-is-now-empty-but-that-breaks-the-extension', -] +attended = ['annotate', 'cat', 'diff', 'export', 'glog', 'log', 'qdiff'] diff -r d17402c07698 -r 4b4b0fb20fc7 tests/test-pager-legacy.t --- a/tests/test-pager-legacy.t Mon Mar 13 21:42:59 2017 -0700 +++ b/tests/test-pager-legacy.t Mon Mar 13 21:43:17 2017 -0700 @@ -50,14 +50,13 @@ paged! 'summary: modify a 9\n' paged! '\n' -BROKEN: should not be paged by default $ hg summary - paged! 'parent: 10:46106edeeb38 tip\n' - paged! ' modify a 10\n' - paged! 'branch: default\n' - paged! 'commit: (clean)\n' - paged! 'update: (current)\n' - paged! 'phases: 11 draft\n' + parent: 10:46106edeeb38 tip + modify a 10 + branch: default + commit: (clean) + update: (current) + phases: 11 draft We can enable the pager on summary: @@ -79,15 +78,14 @@ +a 2 If we completely change the attend list that's respected: -BROKEN: diff should not be paged $ hg --config pager.attend=summary diff -c 2 - paged! 'diff -r f4be7687d414 -r bce265549556 a\n' - paged! '--- a/a\tThu Jan 01 00:00:00 1970 +0000\n' - paged! '+++ b/a\tThu Jan 01 00:00:00 1970 +0000\n' - paged! '@@ -1,2 +1,3 @@\n' - paged! ' a\n' - paged! ' a 1\n' - paged! '+a 2\n' + diff -r f4be7687d414 -r bce265549556 a + --- a/a Thu Jan 01 00:00:00 1970 +0000 + +++ b/a Thu Jan 01 00:00:00 1970 +0000 + @@ -1,2 +1,3 @@ + a + a 1 + +a 2 If 'log' is in attend, then 'history' should also be paged: $ hg history --limit 2 --config pager.attend=log