git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Richard Hansen <rhansen@bbn.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] test-lib.sh: do tests for color support after changing HOME
Date: Tue, 06 Jan 2015 11:06:31 -0800	[thread overview]
Message-ID: <xmqq8uhfpw3s.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1420484054-15948-1-git-send-email-rhansen@bbn.com> (Richard Hansen's message of "Mon, 5 Jan 2015 13:54:14 -0500")

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"

  reply	other threads:[~2015-01-06 19:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-05 18:54 [PATCH] test-lib.sh: do tests for color support after changing HOME Richard Hansen
2015-01-06 19:06 ` Junio C Hamano [this message]
2015-01-06 22:57   ` [PATCH v2 0/2] " Richard Hansen
2015-01-06 22:57     ` [PATCH v2 1/2] use 'test ...' instead of '[ ... ]' Richard Hansen
2015-01-06 22:57     ` [PATCH v2 2/2] test-lib.sh: do tests for color support after changing HOME Richard Hansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq8uhfpw3s.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=rhansen@bbn.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).