On Wed, Apr 12, 2017 at 05:05:58PM -0700, Jun Wu wrote:
> I know it's semi-freeze so new features are not welcome now. But we have got
> enough complaints (pager loses color, draws raw characters, enters
> fullscreen unexpectedly) internally.
>
> Strictly speaking, this is an user error of setting "PAGER" to "less".
> However other software like git works just fine. Therefore I think this is
> an important fix to match git's behavior.

Agreed. Send a v2 addressing Yuya's comments?

>
> Excerpts from Jun Wu's message of 2017-04-12 16:51:05 -0700:
> > # HG changeset patch
> > # User Jun Wu <qu...@fb.com>
> > # Date 1492039375 25200
> > #      Wed Apr 12 16:22:55 2017 -0700
> > # Node ID c8f21c8e1a9a8b5ca97bb87916ede5770d6f7021
> > # Parent  1c398f7f4aa437a7c692025f0434c0564dbf72ff
> > # Available At https://bitbucket.org/quark-zju/hg-draft
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 
> > c8f21c8e1a9a
> > pager: set some environment variables if they're not set
> >
> > Git did this already [1] [2]. We want this behavior too [3].
> >
> > This provides a better default user experience (like, supporting colors) if
> > users have things like "PAGER=less" set, which is not uncommon.
> >
> > [1]: 
> > https://github.com/git/git/blob/6a5ff7acb5965718cc7016c0ab6c601454fd7cde/pager.c#L87
> > [2]: 
> > https://github.com/git/git/blob/6a5ff7acb5965718cc7016c0ab6c601454fd7cde/Makefile#L1545
> > [3]: 
> > https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-March/094780.html
> >
> > diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
> > --- a/mercurial/chgserver.py
> > +++ b/mercurial/chgserver.py
> > @@ -191,6 +191,6 @@ def _newchgui(srcui, csystem, attachio):
> >              return self._csystem(cmd, util.shellenviron(environ), cwd)
> >
> > -        def _runpager(self, cmd):
> > -            self._csystem(cmd, util.shellenviron(), type='pager',
> > +        def _runpager(self, cmd, env=None):
> > +            self._csystem(cmd, util.shellenviron(env), type='pager',
> >                            cmdtable={'attachio': attachio})
> >              return True
> > diff --git a/mercurial/help/pager.txt b/mercurial/help/pager.txt
> > --- a/mercurial/help/pager.txt
> > +++ b/mercurial/help/pager.txt
> > @@ -34,2 +34,11 @@ To globally turn off all attempts to use
> >
> >  which will prevent the pager from running.
> > +
> > +In order to provide a better default user experience, some environment
> > +variables will be set if they are not set. For example, if LESS is not 
> > set, it
> > +will be set to FRX. To override the list of environment variables, set::
> > +
> > +  [pager]
> > +  defaultenv = LESS=FRQXi, LESSKEY=~/.lesskey
> > +
> > +Commas are needed to separate multiple default environment variables.
> > diff --git a/mercurial/ui.py b/mercurial/ui.py
> > --- a/mercurial/ui.py
> > +++ b/mercurial/ui.py
> > @@ -855,4 +855,13 @@ class ui(object):
> >              return
> >
> > +        pagerenv = encoding.environ.copy()
> > +        for entry in self.configlist('pager', 'defaultenv',
> > +                                     ['LESS=FRX', 'LV=-c']):
> > +            if '=' not in entry:
> > +                raise error.Abort(_('invalid pager.defaultenv config: %s')
> > +                                  % entry)
> > +            name, value = entry.split('=', 1)
> > +            pagerenv.setdefault(name, value)
> > +
> >          self.debug('starting pager for command %r\n' % command)
> >          self.flush()
> > @@ -861,5 +870,5 @@ class ui(object):
> >          if util.safehasattr(signal, "SIGPIPE"):
> >              signal.signal(signal.SIGPIPE, _catchterm)
> > -        if self._runpager(pagercmd):
> > +        if self._runpager(pagercmd, pagerenv):
> >              self.pageractive = True
> >              # Preserve the formatted-ness of the UI. This is important
> > @@ -880,5 +889,5 @@ class ui(object):
> >              self.disablepager()
> >
> > -    def _runpager(self, command):
> > +    def _runpager(self, command, env=None):
> >          """Actually start the pager and set up file descriptors.
> >
> > @@ -913,5 +922,6 @@ class ui(object):
> >                  command, shell=shell, bufsize=-1,
> >                  close_fds=util.closefds, stdin=subprocess.PIPE,
> > -                stdout=util.stdout, stderr=util.stderr)
> > +                stdout=util.stdout, stderr=util.stderr,
> > +                env=util.shellenviron(env))
> >          except OSError as e:
> >              if e.errno == errno.ENOENT and not shell:
> > diff --git a/tests/test-pager.t b/tests/test-pager.t
> > --- a/tests/test-pager.t
> > +++ b/tests/test-pager.t
> > @@ -255,2 +255,41 @@ Put annotate in the ignore list for page
> >     9: a 9
> >    10: a 10
> > +
> > +pager.defaultenv controls default environments
> > +
> > +  $ cat > $TESTTMP/printless.py <<EOF
> > +  > import os, sys
> > +  > sys.stdin.read()
> > +  > for name in ['LESS', 'LV']:
> > +  >     sys.stdout.write(('%s=%s\n') % (name, os.environ.get(name, '-')))
> > +  > sys.stdout.flush()
> > +  > EOF
> > +
> > +  $ cat >> $HGRCPATH <<EOF
> > +  > [alias]
> > +  > noop = log -r 0 -T ''
> > +  > [ui]
> > +  > formatted=1
> > +  > [pager]
> > +  > pager = $PYTHON $TESTTMP/printless.py
> > +  > EOF
> > +  $ unset LESS
> > +  $ unset LV
> > +  $ hg noop --pager=on
> > +  LESS=FRX
> > +  LV=-c
> > +  $ hg noop --pager=on --config pager.defaultenv=
> > +  LESS=-
> > +  LV=-
> > +  $ hg noop --pager=on --config pager.defaultenv=LESS=ABCD
> > +  LESS=ABCD
> > +  LV=-
> > +  $ hg noop --pager=on --config pager.defaultenv=UNRELATED=X,LV=ABCD
> > +  LESS=-
> > +  LV=ABCD
> > +  $ hg noop --pager=on --config pager.defaultenv=LV=ABCD,LESS=DCBA
> > +  LESS=DCBA
> > +  LV=ABCD
> > +  $ LESS=EFGH hg noop --pager=on --config 
> > pager.defaultenv=LESS=ABCD,LV=IJKL
> > +  LESS=EFGH
> > +  LV=IJKL
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to