| Submitter | Matt Harbison |
|---|---|
| Date | March 24, 2017, 1:57 a.m. |
| Message ID | <d0c2db2d9f13dca534c5.1490320670@Envy> |
| Download | mbox | patch |
| Permalink | /patch/19615/ |
| State | Accepted |
| Headers | show |
Comments
> On Mar 23, 2017, at 9:57 PM, Matt Harbison <mharbison72@gmail.com> wrote: > > # HG changeset patch > # User Matt Harbison <matt_harbison@yahoo.com> > # Date 1489983573 14400 > # Mon Mar 20 00:19:33 2017 -0400 > # Node ID d0c2db2d9f13dca534c598de050eb1919ef79059 > # Parent df82f375fa005b9c71b463182e6b54aa47fa5999 > pager: fix the invocation of `more` on Windows Queued, thanks much for your work on Windows support. > > After 9335dc6b2a9c, with 'shell' being (mostly) set to False, invoking `more` no > longer worked. Instead, a warning was printed and the pager was disabled. > Invoking `more.com` works. Since a user may have configured 'pager.pager=more', > do this substitution at the end. Surprisingly, `more` does allow for arguments, > so those are preserved. This also allows `more` to work in MSYS. > > Setting 'shell=False' runs the executable via CreateProcess(), which has rather > wonky rules for resolving an executable without an extension [1]. Resolving to > *.com is not among them. Since 'shell=True' yields a cryptic error for a bad > $PAGER, and a *.exe program will work without specifying the extension, sticking > with current 'shell=False' seems like the right thing to do. I don't think > there are any other *.com pagers out there, so this one special case seems OK. > > If somebody wants to do something crazy that requires cmd.exe, I was able to get > normal paged output with 'pager.pager="cmd.exe /c more"'. I assume you can > replace `more` with *.bat, *.vbs or various other creatures listed in $PATHEXT. > > [1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx > > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -844,6 +844,15 @@ > if not pagercmd: > return > > + if pycompat.osname == 'nt': > + # `more` cannot be invoked with shell=False, but `more.com` can. > + # Hide this implementation detail from the user, so we can also get > + # sane bad PAGER behavior. If args are also given, the space in the > + # command line forces shell=True, so that case doesn't need to be > + # handled here. > + if pagercmd == 'more': > + pagercmd = 'more.com' > + > self.debug('starting pager for command %r\n' % command) > self.flush() > self.pageractive = True > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Thu, 23 Mar 2017 21:57:50 -0400, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison <matt_harbison@yahoo.com> > # Date 1489983573 14400 > # Mon Mar 20 00:19:33 2017 -0400 > # Node ID d0c2db2d9f13dca534c598de050eb1919ef79059 > # Parent df82f375fa005b9c71b463182e6b54aa47fa5999 > pager: fix the invocation of `more` on Windows > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -844,6 +844,15 @@ > if not pagercmd: > return > > + if pycompat.osname == 'nt': > + # `more` cannot be invoked with shell=False, but `more.com` can. > + # Hide this implementation detail from the user, so we can also get > + # sane bad PAGER behavior. If args are also given, the space in the > + # command line forces shell=True, so that case doesn't need to be > + # handled here. > + if pagercmd == 'more': > + pagercmd = 'more.com' Given MSYS nor MSYS2 doesn't provide more.exe by default, this seems fine. Can we document this hack in help just in case someone installed more.exe?
On Fri, 24 Mar 2017 10:17:46 -0400, Yuya Nishihara <yuya@tcha.org> wrote: > On Thu, 23 Mar 2017 21:57:50 -0400, Matt Harbison wrote: >> # HG changeset patch >> # User Matt Harbison <matt_harbison@yahoo.com> >> # Date 1489983573 14400 >> # Mon Mar 20 00:19:33 2017 -0400 >> # Node ID d0c2db2d9f13dca534c598de050eb1919ef79059 >> # Parent df82f375fa005b9c71b463182e6b54aa47fa5999 >> pager: fix the invocation of `more` on Windows > >> diff --git a/mercurial/ui.py b/mercurial/ui.py >> --- a/mercurial/ui.py >> +++ b/mercurial/ui.py >> @@ -844,6 +844,15 @@ >> if not pagercmd: >> return >> >> + if pycompat.osname == 'nt': >> + # `more` cannot be invoked with shell=False, but >> `more.com` can. >> + # Hide this implementation detail from the user, so we can >> also get >> + # sane bad PAGER behavior. If args are also given, the >> space in the >> + # command line forces shell=True, so that case doesn't >> need to be >> + # handled here. >> + if pagercmd == 'more': >> + pagercmd = 'more.com' > > Given MSYS nor MSYS2 doesn't provide more.exe by default, this seems > fine. > Can we document this hack in help just in case someone installed > more.exe? I know this is an edge case, but instead of documenting how a user can't just say 'more' and spawn 'more.exe', what do you think about calling util.findexe() if pagercmd == 'more'? That finds 'more.com' too, so things should just work as if they were in cmd.exe.
On Fri, 24 Mar 2017 20:16:30 -0400, Matt Harbison wrote: > On Fri, 24 Mar 2017 10:17:46 -0400, Yuya Nishihara <yuya@tcha.org> wrote: > > On Thu, 23 Mar 2017 21:57:50 -0400, Matt Harbison wrote: > >> + # handled here. > >> + if pagercmd == 'more': > >> + pagercmd = 'more.com' > > > > Given MSYS nor MSYS2 doesn't provide more.exe by default, this seems > > fine. > > Can we document this hack in help just in case someone installed > > more.exe? > > I know this is an edge case, but instead of documenting how a user can't > just say 'more' and spawn 'more.exe', what do you think about calling > util.findexe() if pagercmd == 'more'? That finds 'more.com' too, so > things should just work as if they were in cmd.exe. Yeah, that sounds much better. We can simply do 'if nt and not shell: findexe()'.
Patch
diff --git a/mercurial/ui.py b/mercurial/ui.py --- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -844,6 +844,15 @@ if not pagercmd: return + if pycompat.osname == 'nt': + # `more` cannot be invoked with shell=False, but `more.com` can. + # Hide this implementation detail from the user, so we can also get + # sane bad PAGER behavior. If args are also given, the space in the + # command line forces shell=True, so that case doesn't need to be + # handled here. + if pagercmd == 'more': + pagercmd = 'more.com' + self.debug('starting pager for command %r\n' % command) self.flush() self.pageractive = True