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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 6, 2015, 19:06 UTC
Message-ID: <xmqq8uhfpw3s.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1420484054-15948-1-git-send-email-rhansen@bbn.com>

Richard Hansen <rhansen@bbn.com> writes:

Show 15 quoted lines

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

Show 17 quoted lines

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

Show 7 quoted lines

> +		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?

Show 39 quoted lines

> +	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"