[PATCH] test-lib.sh: do tests for color support after changing HOME

[PATCH] test-lib.sh: do tests for color support after changing HOME

[Richard Hansen]

If ncurses needs ~/.terminfo for the current $TERM, then tput will
succeed before changing HOME to $TRASH_DIRECTORY but fail afterward.
Move the tests that determine whether there is color support after
changing HOME so that color=t is set if and only if tput would succeed
when say_color() is run.

This disables color support for those that need ~/.terminfo for their
TERM, but it's better than filling the screen with:

    tput: unknown terminal "custom-terminal-name-here"

An alternative would be to symlink or copy the user's terminfo
database into $TRASH_DIRECTORY, but this is tricky due to the lack of
a standard name for the terminfo database (for example, instead of a
~/.terminfo directory, NetBSD uses a ~/.terminfo.cdb database file).

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 t/test-lib.sh | 90 +++++++++++++++++++++++++++++++----------------------------
 1 file changed, 47 insertions(+), 43 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9acdc88..65ecbed 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -184,16 +184,8 @@ export _x05 _x40 _z40 LF u200c
 # This test checks if command xyzzy does the right thing...
 # '
 # . ./test-lib.sh
-[ "x$ORIGINAL_TERM" != "xdumb" ] && (
-		TERM=$ORIGINAL_TERM &&
-		export TERM &&
-		[ -t 1 ] &&
-		tput bold >/dev/null 2>&1 &&
-		tput setaf 1 >/dev/null 2>&1 &&
-		tput sgr0 >/dev/null 2>&1
-	) &&
-	color=t
 
+unset color
 while test "$#" -ne 0
 do
 	case "$1" in
@@ -258,40 +250,6 @@ then
 	verbose=t
 fi
 
-if test -n "$color"
-then
-	say_color () {
-		(
-		TERM=$ORIGINAL_TERM
-		export TERM
-		case "$1" in
-		error)
-			tput bold; tput setaf 1;; # bold red
-		skip)
-			tput setaf 4;; # blue
-		warn)
-			tput setaf 3;; # brown/yellow
-		pass)
-			tput setaf 2;; # green
-		info)
-			tput setaf 6;; # cyan
-		*)
-			test -n "$quiet" && return;;
-		esac
-		shift
-		printf "%s" "$*"
-		tput sgr0
-		echo
-		)
-	}
-else
-	say_color() {
-		test -z "$1" && test -n "$quiet" && return
-		shift
-		printf "%s\n" "$*"
-	}
-fi
-
 error () {
 	say_color error "error: $*"
 	GIT_EXIT_OK=t
@@ -857,6 +815,52 @@ HOME="$TRASH_DIRECTORY"
 GNUPGHOME="$HOME/gnupg-home-not-used"
 export HOME GNUPGHOME
 
+# run the tput tests *after* changing HOME (in case ncurses needs
+# ~/.terminfo for $TERM)
+test -n "${color+set}" || [ "x$ORIGINAL_TERM" != "xdumb" ] && (
+		TERM=$ORIGINAL_TERM &&
+		export TERM &&
+		[ -t 1 ] &&
+		tput bold >/dev/null 2>&1 &&
+		tput setaf 1 >/dev/null 2>&1 &&
+		tput sgr0 >/dev/null 2>&1
+	) &&
+	color=t
+
+if test -n "$color"
+then
+	say_color () {
+		(
+		TERM=$ORIGINAL_TERM
+		export TERM
+		case "$1" in
+		error)
+			tput bold; tput setaf 1;; # bold red
+		skip)
+			tput setaf 4;; # blue
+		warn)
+			tput setaf 3;; # brown/yellow
+		pass)
+			tput setaf 2;; # green
+		info)
+			tput setaf 6;; # cyan
+		*)
+			test -n "$quiet" && return;;
+		esac
+		shift
+		printf "%s" "$*"
+		tput sgr0
+		echo
+		)
+	}
+else
+	say_color() {
+		test -z "$1" && test -n "$quiet" && return
+		shift
+		printf "%s\n" "$*"
+	}
+fi
+
 if test -z "$TEST_NO_CREATE_REPO"
 then
 	test_create_repo "$TRASH_DIRECTORY"
-- 
2.2.1

Re: [PATCH] test-lib.sh: do tests for color support after changing HOME

[Junio C Hamano]

Richard Hansen <rhansen@bbn.com> writes:

> If ncurses needs ~/.terminfo for the current $TERM, then tput will
> succeed before changing HOME to $TRASH_DIRECTORY but fail afterward.
> Move the tests that determine whether there is color support after
> changing HOME so that color=t is set if and only if tput would succeed
> when say_color() is run.
>
> This disables color support for those that need ~/.terminfo for their
> TERM, but it's better than filling the screen with:
>
>     tput: unknown terminal "custom-terminal-name-here"
>
> An alternative would be to symlink or copy the user's terminfo
> database into $TRASH_DIRECTORY, but this is tricky due to the lack of
> a standard name for the terminfo database (for example, instead of a
> ~/.terminfo directory, NetBSD uses a ~/.terminfo.cdb database file).

Sounds like a very sensible design trade-off.

> +unset color
>  while test "$#" -ne 0
>  do
>  	case "$1" in
> @@ -258,40 +250,6 @@ then
>  	verbose=t
>  fi
>  
> -if test -n "$color"
> ...
> @@ -857,6 +815,52 @@ HOME="$TRASH_DIRECTORY"
>  GNUPGHOME="$HOME/gnupg-home-not-used"
>  export HOME GNUPGHOME
>  
> +# run the tput tests *after* changing HOME (in case ncurses needs
> +# ~/.terminfo for $TERM)
> +test -n "${color+set}" || [ "x$ORIGINAL_TERM" != "xdumb" ] && (

OK, $color used to be boolean between '' (unset included) and 't',
but now we do this after possibly processing the --no-color
argument, so this is guarded slightly differently from the original.

Makes sense.

> +		TERM=$ORIGINAL_TERM &&
> +		export TERM &&
> +		[ -t 1 ] &&
> +		tput bold >/dev/null 2>&1 &&
> +		tput setaf 1 >/dev/null 2>&1 &&
> +		tput sgr0 >/dev/null 2>&1
> +	) &&

Thanks.

This is a tangent but this patch shows 2 places out of the only
three places we use [ ... ] construct (as opposed to a more
traditionalist "test").  Perhaps we may want to fix them with a
follow-up patch?

> +	color=t
> +
> +if test -n "$color"
> +then
> +	say_color () {
> +		(
> +		TERM=$ORIGINAL_TERM
> +		export TERM
> +		case "$1" in
> +		error)
> +			tput bold; tput setaf 1;; # bold red
> +		skip)
> +			tput setaf 4;; # blue
> +		warn)
> +			tput setaf 3;; # brown/yellow
> +		pass)
> +			tput setaf 2;; # green
> +		info)
> +			tput setaf 6;; # cyan
> +		*)
> +			test -n "$quiet" && return;;
> +		esac
> +		shift
> +		printf "%s" "$*"
> +		tput sgr0
> +		echo
> +		)
> +	}
> +else
> +	say_color() {
> +		test -z "$1" && test -n "$quiet" && return
> +		shift
> +		printf "%s\n" "$*"
> +	}
> +fi
> +
>  if test -z "$TEST_NO_CREATE_REPO"
>  then
>  	test_create_repo "$TRASH_DIRECTORY"

[PATCH v2 0/2] test-lib.sh: do tests for color support after changing HOME

[Richard Hansen]

On 2015-01-06T11:06-08:00, Junio C Hamano wrote:
>> +unset color
>>  while test "$#" -ne 0
>>  do
>>  	case "$1" in
>> @@ -258,40 +250,6 @@ then
>>  	verbose=t
>>  fi
>>  
>> -if test -n "$color"
>> ...
>> @@ -857,6 +815,52 @@ HOME="$TRASH_DIRECTORY"
>>  GNUPGHOME="$HOME/gnupg-home-not-used"
>>  export HOME GNUPGHOME
>>  
>> +# run the tput tests *after* changing HOME (in case ncurses needs
>> +# ~/.terminfo for $TERM)
>> +test -n "${color+set}" || [ "x$ORIGINAL_TERM" != "xdumb" ] && (
>
> OK, $color used to be boolean between '' (unset included) and 't',
> but now we do this after possibly processing the --no-color
> argument, so this is guarded slightly differently from the original.
>
> Makes sense.

I updated the commit message to make this change more obvious.

> This is a tangent but this patch shows 2 places out of the only
> three places we use [ ... ] construct (as opposed to a more
> traditionalist "test").  Perhaps we may want to fix them with a
> follow-up patch?

I added a prequel patch to address this.

Thank you for taking a look,
Richard


Richard Hansen (2):
  use 'test ...' instead of '[ ... ]'
  test-lib.sh: do tests for color support after changing HOME

 t/test-lib.sh | 92 +++++++++++++++++++++++++++++++----------------------------
 1 file changed, 48 insertions(+), 44 deletions(-)

-- 
2.2.1

[PATCH v2 1/2] use 'test ...' instead of '[ ... ]'

[Richard Hansen]

(see Documentation/CodingGuidelines)

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 t/test-lib.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9acdc88..3670eed 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -184,10 +184,10 @@ export _x05 _x40 _z40 LF u200c
 # This test checks if command xyzzy does the right thing...
 # '
 # . ./test-lib.sh
-[ "x$ORIGINAL_TERM" != "xdumb" ] && (
+test "x$ORIGINAL_TERM" != "xdumb" && (
 		TERM=$ORIGINAL_TERM &&
 		export TERM &&
-		[ -t 1 ] &&
+		test -t 1 &&
 		tput bold >/dev/null 2>&1 &&
 		tput setaf 1 >/dev/null 2>&1 &&
 		tput sgr0 >/dev/null 2>&1
@@ -684,7 +684,7 @@ test_done () {
 		then
 			error "Can't use skip_all after running some tests"
 		fi
-		[ -z "$skip_all" ] || skip_all=" # SKIP $skip_all"
+		test -z "$skip_all" || skip_all=" # SKIP $skip_all"
 
 		if test $test_external_has_tap -eq 0
 		then
-- 
2.2.1

[PATCH v2 2/2] test-lib.sh: do tests for color support after changing HOME

[Richard Hansen]

If ncurses needs ~/.terminfo for the current $TERM, then tput will
succeed before changing HOME to $TRASH_DIRECTORY but fail afterward.
Move the tests that determine whether there is color support after
changing HOME so that color=t is set if and only if tput would succeed
when say_color() is run.

Note that color=t is now set after --no-color is processed, so the
condition to set color=t has changed:  it is now set only if
color has not already been set to the empty string by --no-color.

This commit disables color support for those that need ~/.terminfo for
their TERM, but it's better than filling the screen with:

    tput: unknown terminal "custom-terminal-name-here"

An alternative would be to symlink or copy the user's terminfo
database into $TRASH_DIRECTORY, but this is tricky due to the lack of
a standard name for the terminfo database (for example, instead of a
~/.terminfo directory, NetBSD uses a ~/.terminfo.cdb database file).

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 t/test-lib.sh | 90 +++++++++++++++++++++++++++++++----------------------------
 1 file changed, 47 insertions(+), 43 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 3670eed..bb1402d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -184,16 +184,8 @@ export _x05 _x40 _z40 LF u200c
 # This test checks if command xyzzy does the right thing...
 # '
 # . ./test-lib.sh
-test "x$ORIGINAL_TERM" != "xdumb" && (
-		TERM=$ORIGINAL_TERM &&
-		export TERM &&
-		test -t 1 &&
-		tput bold >/dev/null 2>&1 &&
-		tput setaf 1 >/dev/null 2>&1 &&
-		tput sgr0 >/dev/null 2>&1
-	) &&
-	color=t
 
+unset color
 while test "$#" -ne 0
 do
 	case "$1" in
@@ -258,40 +250,6 @@ then
 	verbose=t
 fi
 
-if test -n "$color"
-then
-	say_color () {
-		(
-		TERM=$ORIGINAL_TERM
-		export TERM
-		case "$1" in
-		error)
-			tput bold; tput setaf 1;; # bold red
-		skip)
-			tput setaf 4;; # blue
-		warn)
-			tput setaf 3;; # brown/yellow
-		pass)
-			tput setaf 2;; # green
-		info)
-			tput setaf 6;; # cyan
-		*)
-			test -n "$quiet" && return;;
-		esac
-		shift
-		printf "%s" "$*"
-		tput sgr0
-		echo
-		)
-	}
-else
-	say_color() {
-		test -z "$1" && test -n "$quiet" && return
-		shift
-		printf "%s\n" "$*"
-	}
-fi
-
 error () {
 	say_color error "error: $*"
 	GIT_EXIT_OK=t
@@ -857,6 +815,52 @@ HOME="$TRASH_DIRECTORY"
 GNUPGHOME="$HOME/gnupg-home-not-used"
 export HOME GNUPGHOME
 
+# run the tput tests *after* changing HOME (in case ncurses needs
+# ~/.terminfo for $TERM)
+test -n "${color+set}" || test "x$ORIGINAL_TERM" != "xdumb" && (
+		TERM=$ORIGINAL_TERM &&
+		export TERM &&
+		test -t 1 &&
+		tput bold >/dev/null 2>&1 &&
+		tput setaf 1 >/dev/null 2>&1 &&
+		tput sgr0 >/dev/null 2>&1
+	) &&
+	color=t
+
+if test -n "$color"
+then
+	say_color () {
+		(
+		TERM=$ORIGINAL_TERM
+		export TERM
+		case "$1" in
+		error)
+			tput bold; tput setaf 1;; # bold red
+		skip)
+			tput setaf 4;; # blue
+		warn)
+			tput setaf 3;; # brown/yellow
+		pass)
+			tput setaf 2;; # green
+		info)
+			tput setaf 6;; # cyan
+		*)
+			test -n "$quiet" && return;;
+		esac
+		shift
+		printf "%s" "$*"
+		tput sgr0
+		echo
+		)
+	}
+else
+	say_color() {
+		test -z "$1" && test -n "$quiet" && return
+		shift
+		printf "%s\n" "$*"
+	}
+fi
+
 if test -z "$TEST_NO_CREATE_REPO"
 then
 	test_create_repo "$TRASH_DIRECTORY"
-- 
2.2.1