Re: [PATCH 1 of 2] pager: set some environment variables if they're not set

2017-04-13 Thread Yuya Nishihara 
On Wed, 12 Apr 2017 16:51:05 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu 
> # 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/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)

Instead of introducing defaultenv config, how about always setting
"LESS=FRX LV=-c" unless they are in environ dict? Users can easily override
them by pager command:

  [pager]
  pager = LESS=whatever less

I feel writing environs in the configlist syntax isn't nice.

> @@ -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))

Nit: util.shellenviron() only needs a partial environ dict to override.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 1 of 2] pager: set some environment variables if they're not set

2017-04-12 Thread Jun Wu 
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.

Excerpts from Jun Wu's message of 2017-04-12 16:51:05 -0700:
> # HG changeset patch
> # User Jun Wu 
> # 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 < +  > 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 < +  > [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

[PATCH 1 of 2] pager: set some environment variables if they're not set

2017-04-12 Thread Jun Wu 
# HG changeset patch
# User Jun Wu 
# 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 < 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 < [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