| Submitter | Augie Fackler |
|---|---|
| Date | Feb. 16, 2017, 4:59 p.m. |
| Message ID | <4c2f3f1b67a71401faff.1487264357@arthedain.pit.corp.google.com> |
| Download | mbox | patch |
| Permalink | /patch/18555/ |
| State | Accepted |
| Headers | show |
Comments
On Thu, 16 Feb 2017 11:59:17 -0500, Augie Fackler wrote: > # HG changeset patch > # User Augie Fackler <augie@google.com> > # Date 1486439567 18000 > # Mon Feb 06 22:52:47 2017 -0500 > # Node ID 4c2f3f1b67a71401faff082dbca79a3f212b5499 > # Parent 935dccd057eb61b980901b5c7a806fac9fb55d0f > annotate: migrate to modern pager API > > diff --git a/hgext/pager.py b/hgext/pager.py > --- a/hgext/pager.py > +++ b/hgext/pager.py > @@ -110,4 +110,4 @@ def uisetup(ui): > extensions.wrapfunction(dispatch, '_runcommand', pagecmd) > extensions.afterloaded('color', afterloaded) > > -attended = ['annotate', 'cat', 'diff', 'export', 'glog', 'log', 'qdiff'] > +attended = ['cat', 'diff', 'export', 'glog', 'log', 'qdiff'] > diff --git a/mercurial/commands.py b/mercurial/commands.py > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -361,6 +361,7 @@ def annotate(ui, repo, *pats, **opts): > > Returns 0 on success. > """ > + ui.pager('annotate') > if not pats: > raise error.Abort(_('at least one filename or pattern is required')) Just to make sure. Do we plan to delay ui.pager() call so short error messages (and password prompt, etc.) won't be paged?
On Feb 18, 2017 03:18, "Yuya Nishihara" <yuya@tcha.org> wrote: On Thu, 16 Feb 2017 11:59:17 -0500, Augie Fackler wrote: > # HG changeset patch > # User Augie Fackler <augie@google.com> > # Date 1486439567 18000 > # Mon Feb 06 22:52:47 2017 -0500 > # Node ID 4c2f3f1b67a71401faff082dbca79a3f212b5499 > # Parent 935dccd057eb61b980901b5c7a806fac9fb55d0f > annotate: migrate to modern pager API > > diff --git a/hgext/pager.py b/hgext/pager.py > --- a/hgext/pager.py > +++ b/hgext/pager.py > @@ -110,4 +110,4 @@ def uisetup(ui): > extensions.wrapfunction(dispatch, '_runcommand', pagecmd) > extensions.afterloaded('color', afterloaded) > > -attended = ['annotate', 'cat', 'diff', 'export', 'glog', 'log', 'qdiff'] > +attended = ['cat', 'diff', 'export', 'glog', 'log', 'qdiff'] > diff --git a/mercurial/commands.py b/mercurial/commands.py > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -361,6 +361,7 @@ def annotate(ui, repo, *pats, **opts): > > Returns 0 on success. > """ > + ui.pager('annotate') > if not pats: > raise error.Abort(_('at least one filename or pattern is required')) Just to make sure. Do we plan to delay ui.pager() call so short error messages (and password prompt, etc.) won't be paged? I was wondering the same, but was hoping the pager would be configured to exit if the full text would fit. I have no idea if that's true for most systems out there, though. I don't believe I've manually configured mine that way, so I suspect it's that way be default on our custom Ubuntu. I guess pager on by default will be pretty annoying if that is not a common default configuration, so I really hope it is.
> On Feb 18, 2017, at 3:11 PM, Martin von Zweigbergk <martinvonz@google.com> wrote: > >> > --- a/mercurial/commands.py >> > +++ b/mercurial/commands.py >> > @@ -361,6 +361,7 @@ def annotate(ui, repo, *pats, **opts): >> > >> > Returns 0 on success. >> > """ >> > + ui.pager('annotate') >> > if not pats: >> > raise error.Abort(_('at least one filename or pattern is required')) >> >> Just to make sure. Do we plan to delay ui.pager() call so short error messages >> (and password prompt, etc.) won't be paged? >> > I was wondering the same, but was hoping the pager would be configured to exit if the full text would fit. I have no idea if that's true for most systems out there, though. I don't believe I've manually configured mine that way, so I suspect it's that way be default on our custom Ubuntu. I guess pager on by default will be pretty annoying if that is not a common default configuration, so I really hope it is. I don’t think that’s a safe assumption, sadly. I’ll do a follow-up to try and make sure that things are sane at least at a high level. AF
On Sat, 18 Feb 2017 15:39:08 -0500, Augie Fackler wrote: > > > On Feb 18, 2017, at 3:11 PM, Martin von Zweigbergk <martinvonz@google.com> wrote: > > > >> > --- a/mercurial/commands.py > >> > +++ b/mercurial/commands.py > >> > @@ -361,6 +361,7 @@ def annotate(ui, repo, *pats, **opts): > >> > > >> > Returns 0 on success. > >> > """ > >> > + ui.pager('annotate') > >> > if not pats: > >> > raise error.Abort(_('at least one filename or pattern is required')) > >> > >> Just to make sure. Do we plan to delay ui.pager() call so short error messages > >> (and password prompt, etc.) won't be paged? > >> > > I was wondering the same, but was hoping the pager would be configured to exit if the full text would fit. I have no idea if that's true for most systems out there, though. I don't believe I've manually configured mine that way, so I suspect it's that way be default on our custom Ubuntu. I guess pager on by default will be pretty annoying if that is not a common default configuration, so I really hope it is. > > I don’t think that’s a safe assumption, sadly. I’ll do a follow-up to try and make sure that things are sane at least at a high level. IIRC, "more" exits automatically, but "less" doesn't by default. Also, "less" clears the paged contents on exit.
Excerpts from Yuya Nishihara's message of 2017-02-19 11:04:19 +0900: > On Sat, 18 Feb 2017 15:39:08 -0500, Augie Fackler wrote: > > > > > On Feb 18, 2017, at 3:11 PM, Martin von Zweigbergk <martinvonz@google.com> wrote: > > > > > >> > --- a/mercurial/commands.py > > >> > +++ b/mercurial/commands.py > > >> > @@ -361,6 +361,7 @@ def annotate(ui, repo, *pats, **opts): > > >> > > > >> > Returns 0 on success. > > >> > """ > > >> > + ui.pager('annotate') > > >> > if not pats: > > >> > raise error.Abort(_('at least one filename or pattern is required')) > > >> > > >> Just to make sure. Do we plan to delay ui.pager() call so short error messages > > >> (and password prompt, etc.) won't be paged? > > >> > > > I was wondering the same, but was hoping the pager would be configured to exit if the full text would fit. I have no idea if that's true for most systems out there, though. I don't believe I've manually configured mine that way, so I suspect it's that way be default on our custom Ubuntu. I guess pager on by default will be pretty annoying if that is not a common default configuration, so I really hope it is. > > > > I don’t think that’s a safe assumption, sadly. I’ll do a follow-up to try and make sure that things are sane at least at a high level. > > IIRC, "more" exits automatically, but "less" doesn't by default. Also, "less" > clears the paged contents on exit. less can be configured to not clear the screen (-X), and exit automatically (-F). And another common flag is -R, which will allow colors. git used to just hard-code LESS=FRX as the default in pager.c [1]. That got moved to an override-able build variable by [2]. I guess we could do the same, provide "LESS=FRX" as the default to be more user-friendly. [1]: https://github.com/git/git/blob/c3b1e8d85133e2a19d372b7c166d5b49fcbbfef2/pager.c#L70 [2]: https://github.com/git/git/commit/995bc22d7f8c611e342095a211065f8585a08e65
> On Feb 18, 2017, at 9:32 PM, Jun Wu <quark@fb.com> wrote: > > Excerpts from Yuya Nishihara's message of 2017-02-19 11:04:19 +0900: >> On Sat, 18 Feb 2017 15:39:08 -0500, Augie Fackler wrote: >>> >>>> On Feb 18, 2017, at 3:11 PM, Martin von Zweigbergk <martinvonz@google.com> wrote: >>>> >>>>>> --- a/mercurial/commands.py >>>>>> +++ b/mercurial/commands.py >>>>>> @@ -361,6 +361,7 @@ def annotate(ui, repo, *pats, **opts): >>>>>> >>>>>> Returns 0 on success. >>>>>> """ >>>>>> + ui.pager('annotate') >>>>>> if not pats: >>>>>> raise error.Abort(_('at least one filename or pattern is required')) >>>>> >>>>> Just to make sure. Do we plan to delay ui.pager() call so short error messages >>>>> (and password prompt, etc.) won't be paged? >>>>> >>>> I was wondering the same, but was hoping the pager would be configured to exit if the full text would fit. I have no idea if that's true for most systems out there, though. I don't believe I've manually configured mine that way, so I suspect it's that way be default on our custom Ubuntu. I guess pager on by default will be pretty annoying if that is not a common default configuration, so I really hope it is. >>> >>> I don’t think that’s a safe assumption, sadly. I’ll do a follow-up to try and make sure that things are sane at least at a high level. >> >> IIRC, "more" exits automatically, but "less" doesn't by default. Also, "less" >> clears the paged contents on exit. > > less can be configured to not clear the screen (-X), and exit automatically > (-F). And another common flag is -R, which will allow colors. > > git used to just hard-code LESS=FRX as the default in pager.c [1]. That got > moved to an override-able build variable by [2]. > > I guess we could do the same, provide "LESS=FRX" as the default to be more > user-friendly. This is a great idea for built packages, but in the sources the default should be more(1) because that’s the only pager we can be sure to find. (I expect our packages on debian would default to sensible-pager, packages on OS X would default to less with FRX set, etc) > > [1]: https://github.com/git/git/blob/c3b1e8d85133e2a19d372b7c166d5b49fcbbfef2/pager.c#L70 > [2]: https://github.com/git/git/commit/995bc22d7f8c611e342095a211065f8585a08e65
Patch
diff --git a/hgext/pager.py b/hgext/pager.py --- a/hgext/pager.py +++ b/hgext/pager.py @@ -110,4 +110,4 @@ def uisetup(ui): extensions.wrapfunction(dispatch, '_runcommand', pagecmd) extensions.afterloaded('color', afterloaded) -attended = ['annotate', 'cat', 'diff', 'export', 'glog', 'log', 'qdiff'] +attended = ['cat', 'diff', 'export', 'glog', 'log', 'qdiff'] diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -361,6 +361,7 @@ def annotate(ui, repo, *pats, **opts): Returns 0 on success. """ + ui.pager('annotate') if not pats: raise error.Abort(_('at least one filename or pattern is required')) diff --git a/tests/test-pager.t b/tests/test-pager.t --- a/tests/test-pager.t +++ b/tests/test-pager.t @@ -164,3 +164,43 @@ Pager should not override the exit code $ hg fortytwo --pager=on paged! '42\n' [42] + +A command that asks for paging using ui.pager() directly works: + $ hg blame a + paged! ' 0: a\n' + paged! ' 1: a 1\n' + paged! ' 2: a 2\n' + paged! ' 3: a 3\n' + paged! ' 4: a 4\n' + paged! ' 5: a 5\n' + paged! ' 6: a 6\n' + paged! ' 7: a 7\n' + paged! ' 8: a 8\n' + paged! ' 9: a 9\n' + paged! '10: a 10\n' +but not with HGPLAIN + $ HGPLAIN=1 hg blame a + 0: a + 1: a 1 + 2: a 2 + 3: a 3 + 4: a 4 + 5: a 5 + 6: a 6 + 7: a 7 + 8: a 8 + 9: a 9 + 10: a 10 +explicit flags work too: + $ hg blame --pager=no a + 0: a + 1: a 1 + 2: a 2 + 3: a 3 + 4: a 4 + 5: a 5 + 6: a 6 + 7: a 7 + 8: a 8 + 9: a 9 + 10: a 10