| Submitter | Jun Wu |
|---|---|
| Date | March 14, 2016, 8:33 p.m. |
| Message ID | <d931d9d34b74ae2d4ed1.1457987629@x1c> |
| Download | mbox | patch |
| Permalink | /patch/13886/ |
| State | Superseded |
| Delegated to: | Yuya Nishihara |
| Headers | show |
Comments
Jun Wu <quark@fb.com> writes: > # HG changeset patch > # User Jun Wu <quark@fb.com> > # Date 1457967799 0 > # Mon Mar 14 15:03:19 2016 +0000 > # Node ID d931d9d34b74ae2d4ed16737c98bfe6b56ec79e5 > # Parent e14741b16063879d0d4d67a4ad5f0129b8f4f769 > pager: skip uisetup if chg is detected > > chg has its own pager implementation that it wants to skip pager's uisetup. > It is currently done by redirecting stdout to /dev/null, which has unintended > side effects. This patch makes pager aware of chg and skip uisetup directly > from pager. We may want to merge chg and pager's pager implementation to > make this unnecessary in the future. > > diff --git a/hgext/pager.py b/hgext/pager.py > --- a/hgext/pager.py > +++ b/hgext/pager.py > @@ -117,6 +117,10 @@ > if '--debugger' in sys.argv or not ui.formatted(): > return > > + # chg has its own pager implementation > + if 'chgunix' in dispatch._earlygetopt('--cmdserver', sys.argv): > + return > + Would this be reason enough to put pager in core so we wouldn't need different implementations?
On 03/14/2016 08:37 PM, Sean Farley wrote: > Would this be reason enough to put pager in core so we wouldn't need > different implementations? The reason that chg needs a different implementation is the pager must be executed by the client. Otherwise it's likely to have trouble with "terminal foreground job", Ctrl+Z etc. To be more precise, the pager is currently using subprocess.Popen to create process at server side. chg needs it to be "remotely" executed at client side. I think Yuya has plans to unify the implementations somehow. See https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-March/081393.html
Jun Wu <quark@fb.com> writes: > On 03/14/2016 08:37 PM, Sean Farley wrote: >> Would this be reason enough to put pager in core so we wouldn't need >> different implementations? > > The reason that chg needs a different implementation is the pager must be > executed by the client. Otherwise it's likely to have trouble with > "terminal foreground job", Ctrl+Z etc. > > To be more precise, the pager is currently using subprocess.Popen to create > process at server side. chg needs it to be "remotely" executed at client > side. > > I think Yuya has plans to unify the implementations somehow. See > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-March/081393.html Yes, ok, this is more of what I was hoping (thanks Yuya for the ideas).
On 03/14/2016 08:33 PM, Jun Wu wrote: > + if 'chgunix' in dispatch._earlygetopt('--cmdserver', sys.argv): Sorry but this is incorrect. _earlygetopt requires an array. It should be: > + if 'chgunix' in dispatch._earlygetopt(['--cmdserver'], sys.argv):
On Tue, 15 Mar 2016 01:26:10 +0000, Jun Wu wrote: > On 03/14/2016 08:33 PM, Jun Wu wrote: > > + if 'chgunix' in dispatch._earlygetopt('--cmdserver', sys.argv): > > Sorry but this is incorrect. _earlygetopt requires an array. It should be: > > > + if 'chgunix' in dispatch._earlygetopt(['--cmdserver'], sys.argv): and _earlygetopt() modifies sys.argv, which is wrong.
Patch
diff --git a/hgext/pager.py b/hgext/pager.py --- a/hgext/pager.py +++ b/hgext/pager.py @@ -117,6 +117,10 @@ if '--debugger' in sys.argv or not ui.formatted(): return + # chg has its own pager implementation + if 'chgunix' in dispatch._earlygetopt('--cmdserver', sys.argv): + return + def pagecmd(orig, ui, options, cmd, cmdfunc): p = ui.config("pager", "pager", os.environ.get("PAGER")) usepager = False