pager: set LV=-c alongside LESS=FRSX
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 pager: set LV=-c alongside LESS=FRSX
On systems with lv configured as the preferred pager (i.e., DEFAULT_PAGER=lv at build time, or PAGER=lv exported in the environment) git commands that use color show control codes instead of color in the pager:
$ git diff ^[[1mdiff --git a/.mailfilter b/.mailfilter^[[m ^[[1mindex aa4f0b2..17e113e 100644^[[m ^[[1m--- a/.mailfilter^[[m ^[[1m+++ b/.mailfilter^[[m ^[[36m@@ -1,11 +1,58 @@^[[m
"less" avoids this problem because git uses the LESS environment variable to pass the -R option ('output ANSI color escapes in raw form') by default. Use the LV environment variable to pass 'lv' the -c option ('allow ANSI escape sequences for text decoration / color') to fix it for lv, too.
Noticed when the default value for color.ui flipped to 'auto' in v1.8.4-rc0~36^2~1 (2013-06-10).
---
Olaf Meeuwissen wrote[1]:
Quoted 2 lines
Yes, it's called LV and documented in the lv(1) manual page. Simply search for 'env' ;-)
Ah, thanks. How about this patch?
[1] http://bugs.debian.org/730527
Documentation/config.txt | 4 ++++ git-sh-setup.sh | 3 ++- pager.c | 11 +++++++++-- perl/Git/SVN/Log.pm | 1 + t/t7006-pager.sh | 12 ++++++++++++ 5 files changed, 28 insertions(+), 3 deletions(-)
Documentation/config.txt +4 -0
@@ -567,6 +567,10 @@ be passed to the shell by Git, which will translate the final command to `LESS=FRSX less -+S`. The environment tells the command to set the `S` option to chop long lines but the command line resets it to the default to fold long lines. ++ +Likewise, when the `LV` environment variable is unset, Git sets it +to `-c`. You can override this setting by exporting `LV` with +another value or setting `core.pager` to `lv +c`. core.whitespace:: A comma separated list of common whitespace problems to
git-sh-setup.sh +2 -1
@@ -159,7 +159,8 @@ git_pager() { GIT_PAGER=cat fi : ${LESS=-FRSX} - export LESS + : ${LV=-c} + export LESS LV eval "$GIT_PAGER" '"$@"' }
pager.c +9 -2
@@ -80,8 +80,15 @@ void setup_pager(void) pager_process.use_shell =1 ; pager_process.argv = pager_argv; pager_process.in =-1 ; -if (!getenv ("LESS" )) { -static const char *env[] = {"LESS=FRSX" ,NULL }; +if (!getenv ("LESS" ) || !getenv ("LV" )) { +static const char *env[3 ]; +int i =0 ; + +if (!getenv ("LESS" )) + env[i++] ="LESS=FRSX" ; +if (!getenv ("LV" )) + env[i++] ="LV=-c" ; + env[i] =NULL ; pager_process.env = env; }if (start_command (&pager_process))
perl/Git/SVN/Log.pm +1 -0
@@ -117,6 +117,7 @@ sub run_pager { } open STDIN, '<&', $rfd or fatal "Can't redirect stdin: $!"; $ENV{LESS} ||= 'FRSX'; + $ENV{LV} ||= '-c'; exec $pager or fatal "Can't run pager: $! ($pager)"; }
t/t7006-pager.sh +12 -0
@@ -37,6 +37,18 @@ test_expect_failure TTY 'pager runs from subdir' ' test_cmp expected actual ' +test_expect_success TTY 'LESS and LV envvars are set for pagination' ' + ( + sane_unset LESS LV && + PAGER="env >pager-env.out" && + export PAGER && + + test_terminal git log + ) && + grep ^LESS= pager-env.out && + grep ^LV= pager-env.out +' + test_expect_success TTY 'some commands do not use a pager' ' rm -f paginated.out && test_terminal git rev-list HEAD &&
Signature · 1 line
1.8.5.1
Quoted 23 lines · Jonathan Nieder
On systems with lv configured as the preferred pager (i.e., DEFAULT_PAGER=lv at build time, or PAGER=lv exported in the environment) git commands that use color show control codes instead of color in the pager:
$ git diff ^[[1mdiff --git a/.mailfilter b/.mailfilter^[[m ^[[1mindex aa4f0b2..17e113e 100644^[[m ^[[1m--- a/.mailfilter^[[m ^[[1m+++ b/.mailfilter^[[m ^[[36m@@ -1,11 +1,58 @@^[[m"less" avoids this problem because git uses the LESS environment variable to pass the -R option ('output ANSI color escapes in raw form') by default. Use the LV environment variable to pass 'lv' the -c option ('allow ANSI escape sequences for text decoration / color') to fix it for lv, too.
Noticed when the default value for color.ui flipped to 'auto' in v1.8.4-rc0~36^2~1 (2013-06-10).
Reported-by Olaf Meeuwissen <[email protected]>Signed-off-by Jonathan Nieder <[email protected]>
Quoted 7 lines
---
Olaf Meeuwissen wrote[1]:Quoted 2 lines
Yes, it's called LV and documented in the lv(1) manual page. Simply search for 'env' ;-)
Ah, thanks. How about this patch?
I'm not qualified to judge the appropriateness of your patch, but if it achieves activating the -c option of lv when that is the pager in effect then that should work for the bulk of the use cases, I'd think.
Quoted 85 lines
[1] http://bugs.debian.org/730527
Documentation/config.txt | 4 ++++ git-sh-setup.sh | 3 ++- pager.c | 11 +++++++++-- perl/Git/SVN/Log.pm | 1 + t/t7006-pager.sh | 12 ++++++++++++ 5 files changed, 28 insertions(+), 3 deletions(-)Documentation/config.txt +4
-0@@ -567,6 +567,10 @@ be passed to the shell by Git, which will translate the final command to `LESS=FRSX less -+S`. The environment tells the command to set the `S` option to chop long lines but the command line resets it to the default to fold long lines. ++ +Likewise, when the `LV` environment variable is unset, Git sets it +to `-c`. You can override this setting by exporting `LV` with +another value or setting `core.pager` to `lv +c`. core.whitespace:: A comma separated list of common whitespace problems togit-sh-setup.sh +2
-1@@ -159,7 +159,8 @@ git_pager() { GIT_PAGER=cat fi : ${LESS=-FRSX} - export LESS + : ${LV=-c} + export LESS LV eval "$GIT_PAGER" '"$@"' }pager.c +9
-2@@ -80,8 +80,15 @@ void setup_pager(void) pager_process.use_shell =1 ; pager_process.argv = pager_argv; pager_process.in =-1 ; -if (!getenv ("LESS" )) { -static const char *env[] = {"LESS=FRSX" ,NULL }; +if (!getenv ("LESS" ) || !getenv ("LV" )) { +static const char *env[3 ]; +int i =0 ; + +if (!getenv ("LESS" )) + env[i++] ="LESS=FRSX" ; +if (!getenv ("LV" )) + env[i++] ="LV=-c" ; + env[i] =NULL ; pager_process.env = env; }if (start_command (&pager_process))perl/Git/SVN/Log.pm +1
-0@@ -117,6 +117,7 @@ sub run_pager { } open STDIN, '<&', $rfd or fatal "Can't redirect stdin: $!"; $ENV{LESS} ||= 'FRSX'; + $ENV{LV} ||= '-c'; exec $pager or fatal "Can't run pager: $! ($pager)"; }t/t7006-pager.sh +5
-0@@ -37,6 +37,18 @@ test_expect_failure TTY 'pager runs from subdir' ' test_cmp expected actual ' +test_expect_success TTY 'LESS and LV envvars are set for pagination' ' + ( + sane_unset LESS LV && + PAGER="env >pager-env.out" && + export PAGER &&
t/t7006-pager.sh test_expect_failure TTY 'pager runs from subdir' ' · line 54
Quoted 10 lines
t/t7006-pager.sh +7
-0+ + test_terminal git log + ) && + grep ^LESS= pager-env.out && + grep ^LV= pager-env.out +' + test_expect_success TTY 'some commands do not use a pager' ' rm -f paginated.out && test_terminal git rev-list HEAD &&
Hope this helps,
Signature · 3 lines
Olaf Meeuwissen, LPIC-2 FLOSS Engineer -- AVASYS CORPORATION
FSF Associate Member #1962 Help support software freedom
http://www.fsf.org/jf?referrer=1962Quoted 26 lines · Jonathan Nieder
On systems with lv configured as the preferred pager (i.e., DEFAULT_PAGER=lv at build time, or PAGER=lv exported in the environment) git commands that use color show control codes instead of color in the pager:
$ git diff ^[[1mdiff --git a/.mailfilter b/.mailfilter^[[m ^[[1mindex aa4f0b2..17e113e 100644^[[m ^[[1m--- a/.mailfilter^[[m ^[[1m+++ b/.mailfilter^[[m ^[[36m@@ -1,11 +1,58 @@^[[m"less" avoids this problem because git uses the LESS environment variable to pass the -R option ('output ANSI color escapes in raw form') by default. Use the LV environment variable to pass 'lv' the -c option ('allow ANSI escape sequences for text decoration / color') to fix it for lv, too.
Noticed when the default value for color.ui flipped to 'auto' in v1.8.4-rc0~36^2~1 (2013-06-10).
Reported-by Olaf Meeuwissen <[email protected]>Signed-off-by Jonathan Nieder <[email protected]>---
Olaf Meeuwissen wrote[1]:
Quoted 6 lines
Quoted 2 lines
Yes, it's called LV and documented in the lv(1) manual page. Simply search for 'env' ;-)
Ah, thanks. How about this patch?
Looks good; though I have to wonder two (and a half) things:
- Scripted Porcelains get LESS=-FRSX while C Porcelains get LESS=FRSX as the default (the leading dash being the difference), which looks somewhat inconsistent. Not a new problem, though.
- Can we generalize this a bit so that a builder can pass a list of var=val pairs and demote the existing LESS=FRSX to just a canned setting of such a mechanism?
- Can such a code be reused to make this into a runtime setting, even? Would it be worth the complexity?
Thanks.
Quoted 93 lines
Documentation/config.txt | 4 ++++ git-sh-setup.sh | 3 ++- pager.c | 11 +++++++++-- perl/Git/SVN/Log.pm | 1 + t/t7006-pager.sh | 12 ++++++++++++ 5 files changed, 28 insertions(+), 3 deletions(-)Documentation/config.txt +4
-0@@ -567,6 +567,10 @@ be passed to the shell by Git, which will translate the final command to `LESS=FRSX less -+S`. The environment tells the command to set the `S` option to chop long lines but the command line resets it to the default to fold long lines. ++ +Likewise, when the `LV` environment variable is unset, Git sets it +to `-c`. You can override this setting by exporting `LV` with +another value or setting `core.pager` to `lv +c`. core.whitespace:: A comma separated list of common whitespace problems togit-sh-setup.sh +2
-1@@ -159,7 +159,8 @@ git_pager() { GIT_PAGER=cat fi : ${LESS=-FRSX} - export LESS + : ${LV=-c} + export LESS LV eval "$GIT_PAGER" '"$@"' }pager.c +9
-2@@ -80,8 +80,15 @@ void setup_pager(void) pager_process.use_shell =1 ; pager_process.argv = pager_argv; pager_process.in =-1 ; -if (!getenv ("LESS" )) { -static const char *env[] = {"LESS=FRSX" ,NULL }; +if (!getenv ("LESS" ) || !getenv ("LV" )) { +static const char *env[3 ]; +int i =0 ; + +if (!getenv ("LESS" )) + env[i++] ="LESS=FRSX" ; +if (!getenv ("LV" )) + env[i++] ="LV=-c" ; + env[i] =NULL ; pager_process.env = env; }if (start_command (&pager_process))perl/Git/SVN/Log.pm +1
-0@@ -117,6 +117,7 @@ sub run_pager { } open STDIN, '<&', $rfd or fatal "Can't redirect stdin: $!"; $ENV{LESS} ||= 'FRSX'; + $ENV{LV} ||= '-c'; exec $pager or fatal "Can't run pager: $! ($pager)"; }t/t7006-pager.sh +12
-0@@ -37,6 +37,18 @@ test_expect_failure TTY 'pager runs from subdir' ' test_cmp expected actual ' +test_expect_success TTY 'LESS and LV envvars are set for pagination' ' + ( + sane_unset LESS LV && + PAGER="env >pager-env.out" && + export PAGER && + + test_terminal git log + ) && + grep ^LESS= pager-env.out && + grep ^LV= pager-env.out +' + test_expect_success TTY 'some commands do not use a pager' ' rm -f paginated.out && test_terminal git rev-list HEAD &&
Quoted 4 lines · Junio C Hamano
- Scripted Porcelains get LESS=-FRSX while C Porcelains get LESS=FRSX as the default (the leading dash being the difference), which looks somewhat inconsistent. Not a new problem, though.
Even the less manpage is inconsistent about whether $LESS should start with a dash (there are examples with and without it). Implementation-wise, less uses the same function to process an option argument (including the leading dash) and the value of $LESS, so the form with the leading dash is probably preferred.
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
t/t7006-pager.sh test_expect_failure TTY 'pager runs from subdir' ' · line 51
Quoted 12 lines · Jonathan Nieder
t/t7006-pager.sh +12
-0+test_expect_success TTY 'LESS and LV envvars are set for pagination' ' + ( + sane_unset LESS LV && + PAGER="env >pager-env.out" && + export PAGER && + + test_terminal git log + ) && + grep ^LESS= pager-env.out && + grep ^LV= pager-env.out +' +
On the Ubuntu PPA builders, I’m seeing this new test fail with SIGPIPE about half the time:
died of signal 13 at /build/buildd/git-1.9~rc1/t/test-terminal.perl line 33. not ok 6 - LESS and LV envvars are set for pagination
Although the test seems to work locally for me, I can reproduce a similar failure just by running
$ GIT_PAGER='env >pager-env.out' ./test-terminal.perl git log died of signal 13 at ./test-terminal.perl line 33.
Anders
PATCH 1/1 pager test: make fake pager consume all its input
Otherwise there is a race: if 'git log' finishes writing before the pager terminates and closes the pipe, all is well, and if the pager finishes quickly enough then 'git log' terminates with SIGPIPE.
died of signal 13 at /build/buildd/git-1.9~rc1/t/test-terminal.perl line 33. not ok 6 - LESS and LV envvars are set for pagination
Noticed on Ubuntu PPA builders, where the race was lost about half the time. Compare v1.7.0.2~6^2 (tests: Fix race condition in t7006-pager, 2010-02-22).
---
Quoted 6 lines · Anders Kaseorg
Quoted 4 lines · Jonathan Nieder
(quoted diff) +4
-0+ PAGER="env >pager-env.out" && + export PAGER && + + test_terminal git log
[...]
Quoted 5 lines
On the Ubuntu PPA builders, I’m seeing this new test fail with SIGPIPE about half the time:
died of signal 13 at /build/buildd/git-1.9~rc1/t/test-terminal.perl line 33. not ok 6 - LESS and LV envvars are set for pagination
Good catch. Sorry for the trouble.
t/t7006-pager.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
t/t7006-pager.sh +1 -1
@@ -40,7 +40,7 @@ test_expect_failure TTY 'pager runs from subdir' ' test_expect_success TTY 'LESS and LV envvars are set for pagination' ' ( sane_unset LESS LV && - PAGER="env >pager-env.out" && + PAGER="env >pager-env.out; wc" && export PAGER && test_terminal git log
Signature · 1 line
1.9.rc1.175.g0b1dcb5
Re: [PATCH jn/pager-lv-default-env] pager test: make fake pager consume all its input
Quoted 7 lines · Jonathan Nieder
Otherwise there is a race: if 'git log' finishes writing before the pager terminates and closes the pipe, all is well, and if the pager finishes quickly enough then 'git log' terminates with SIGPIPE.
died of signal 13 at /build/buildd/git-1.9~rc1/t/test-terminal.perl line 33. not ok 6 - LESS and LV envvars are set for pagination
Quoted 7 lines
Noticed on Ubuntu PPA builders, where the race was lost about half the time. Compare v1.7.0.2~6^2 (tests: Fix race condition in t7006-pager, 2010-02-22).
Reported-by Anders Kaseorg <[email protected]>Signed-off-by Jonathan Nieder <[email protected]>---
Thanks. The use of "wc" looks cute (I would have used "cat").
Quoted 32 lines
Quoted 6 lines · Anders Kaseorg
Quoted 4 lines · Jonathan Nieder
(quoted diff) +4
-0+ PAGER="env >pager-env.out" && + export PAGER && + + test_terminal git log[...]
Quoted 5 lines
On the Ubuntu PPA builders, I’m seeing this new test fail with SIGPIPE about half the time:
died of signal 13 at /build/buildd/git-1.9~rc1/t/test-terminal.perl line 33. not ok 6 - LESS and LV envvars are set for pagination
Good catch. Sorry for the trouble.
t/t7006-pager.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)t/t7006-pager.sh +1
-1@@ -40,7 +40,7 @@ test_expect_failure TTY 'pager runs from subdir' ' test_expect_success TTY 'LESS and LV envvars are set for pagination' ' ( sane_unset LESS LV && - PAGER="env >pager-env.out" && + PAGER="env >pager-env.out; wc" && export PAGER && test_terminal git log