[PATCH] 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).

Reported-by: Olaf Meeuwissen <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Olaf Meeuwissen wrote[1]:

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

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a405806..ed59853 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

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

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 190a539..fffa3c7 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh

@@ -159,7 +159,8 @@ git_pager() {
 		GIT_PAGER=cat
 	fi
 	: ${LESS=-FRSX}
-	export LESS
+	: ${LV=-c}
+	export LESS LV
 
 	eval "$GIT_PAGER" '"$@"'
 }

diff --git a/pager.c b/pager.c
index 345b0bc..0cc75a8 100644
--- a/pager.c
+++ b/pager.c

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

diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm
index 3f8350a..34f2869 100644
--- a/perl/Git/SVN/Log.pm
+++ b/perl/Git/SVN/Log.pm

@@ -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)";
 }
 

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index ff25908..7fe3367 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh

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

-- 
1.8.5.1

Jonathan Nieder writes:

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 <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Olaf Meeuwissen wrote[1]:

quoted

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.

Hope this helps,
-- 
Olaf Meeuwissen, LPIC-2           FLOSS Engineer -- AVASYS CORPORATION
FSF Associate Member #1962               Help support software freedom
                 http://www.fsf.org/jf?referrer=1962

On 01/06/2014 09:14 PM, Jonathan Nieder wrote:

+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

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

Reported-by: Anders Kaseorg <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Anders Kaseorg wrote:

On 01/06/2014 09:14 PM, Jonathan Nieder wrote:

quoted

+		PAGER="env >pager-env.out" &&
+		export PAGER &&
+
+		test_terminal git log

[...]

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

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 7fe3367..b9365b4 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh

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

-- 
1.9.rc1.175.g0b1dcb5