[PATCH] t9001: set TERM=dumb to prevent ANSI output

[PATCH] t9001: set TERM=dumb to prevent ANSI output

[Johannes Schindelin]

At least on one of my machines, Term::ReadLine tries to be clever and
sends \x1b\x5b1034h at the end of the script when TERM is set to 'xterm'.

To prevent that, force TERM=dumb just for the test.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t9001-send-email.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index cbbfa9c..d6fcac3 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -68,7 +68,7 @@ Result: OK
 EOF
 
 test_expect_success 'Show all headers' '
-	git send-email \
+	TERM=dumb git send-email \
 		--dry-run \
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
-- 
1.5.4.3.571.g9aec3.dirty

Re: [PATCH] t9001: set TERM=dumb to prevent ANSI output

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> At least on one of my machines, Term::ReadLine tries to be clever and
> sends \x1b\x5b1034h at the end of the script when TERM is set to 'xterm'.
>
> To prevent that, force TERM=dumb just for the test.

We try to set up a pretty vanilla environment for test repeatability in
t/test-lib.sh, and I suspect we would simply want to do this over there.

Wouldn't this make more sense?

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 87a5ea4..7f6331a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -7,8 +7,9 @@
 LANG=C
 LC_ALL=C
 PAGER=cat
+TERM=dumb
 TZ=UTC
-export LANG LC_ALL PAGER TZ
+export LANG LC_ALL PAGER TERM TZ
 EDITOR=:
 VISUAL=:
 unset GIT_EDITOR

Re: [PATCH] t9001: set TERM=dumb to prevent ANSI output

[Johannes Schindelin]

Hi,

On Thu, 6 Mar 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > At least on one of my machines, Term::ReadLine tries to be clever and
> > sends \x1b\x5b1034h at the end of the script when TERM is set to 'xterm'.
> >
> > To prevent that, force TERM=dumb just for the test.
> 
> We try to set up a pretty vanilla environment for test repeatability in
> t/test-lib.sh, and I suspect we would simply want to do this over there.
> 
> Wouldn't this make more sense?

Sure!

Ciao,
Dscho

Re: [PATCH] t9001: set TERM=dumb to prevent ANSI output

[Christian Couder]

Le vendredi 7 mars 2008, Junio C Hamano a écrit :
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > At least on one of my machines, Term::ReadLine tries to be clever and
> > sends \x1b\x5b1034h at the end of the script when TERM is set to
> > 'xterm'.
> >
> > To prevent that, force TERM=dumb just for the test.
>
> We try to set up a pretty vanilla environment for test repeatability in
> t/test-lib.sh, and I suspect we would simply want to do this over there.
>
> Wouldn't this make more sense?
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 87a5ea4..7f6331a 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -7,8 +7,9 @@
>  LANG=C
>  LC_ALL=C
>  PAGER=cat
> +TERM=dumb
>  TZ=UTC
> -export LANG LC_ALL PAGER TZ
> +export LANG LC_ALL PAGER TERM TZ
>  EDITOR=:
>  VISUAL=:
>  unset GIT_EDITOR

This may not work well with colored output.
In test-lib.sh line 62 there is :

[ "x$TERM" != "xdumb" ] &&
	[ -t 1 ] &&
	tput bold >/dev/null 2>&1 &&
	tput setaf 1 >/dev/null 2>&1 &&
	tput sgr0 >/dev/null 2>&1 &&
	color=t

Thanks,
Christian.

Re: [PATCH] t9001: set TERM=dumb to prevent ANSI output

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> Le vendredi 7 mars 2008, Junio C Hamano a écrit :
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > At least on one of my machines, Term::ReadLine tries to be clever and
>> > sends \x1b\x5b1034h at the end of the script when TERM is set to
>> > 'xterm'.
>> >
>> > To prevent that, force TERM=dumb just for the test.
>>
>> We try to set up a pretty vanilla environment for test repeatability in
>> t/test-lib.sh, and I suspect we would simply want to do this over there.
>>
>> Wouldn't this make more sense?
>>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 87a5ea4..7f6331a 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -7,8 +7,9 @@
>>  LANG=C
>>  LC_ALL=C
>>  PAGER=cat
>> +TERM=dumb
>>  TZ=UTC
>> -export LANG LC_ALL PAGER TZ
>> +export LANG LC_ALL PAGER TERM TZ
>>  EDITOR=:
>>  VISUAL=:
>>  unset GIT_EDITOR
>
> This may not work well with colored output.
> In test-lib.sh line 62 there is :
>
> [ "x$TERM" != "xdumb" ] &&
> 	[ -t 1 ] &&
> 	tput bold >/dev/null 2>&1 &&
> 	tput setaf 1 >/dev/null 2>&1 &&
> 	tput sgr0 >/dev/null 2>&1 &&
> 	color=t

Ahh.  Sorry about that.

 t/test-lib.sh |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index c0c5e21..8a8b33e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -3,12 +3,23 @@
 # Copyright (c) 2005 Junio C Hamano
 #
 
+# Grab if the true terminal is capable of color before resetting
+# TERM to dumb for repeatability
+
+[ "x$TERM" != "xdumb" ] &&
+	[ -t 1 ] &&
+	tput bold >/dev/null 2>&1 &&
+	tput setaf 1 >/dev/null 2>&1 &&
+	tput sgr0 >/dev/null 2>&1 &&
+	color=t
+
 # For repeatability, reset the environment to known value.
 LANG=C
 LC_ALL=C
 PAGER=cat
 TZ=UTC
-export LANG LC_ALL PAGER TZ
+TERM=dumb
+export LANG LC_ALL PAGER TERM TZ
 EDITOR=:
 VISUAL=:
 unset GIT_EDITOR
@@ -59,13 +70,6 @@ esac
 # '
 # . ./test-lib.sh
 
-[ "x$TERM" != "xdumb" ] &&
-	[ -t 1 ] &&
-	tput bold >/dev/null 2>&1 &&
-	tput setaf 1 >/dev/null 2>&1 &&
-	tput sgr0 >/dev/null 2>&1 &&
-	color=t

Re: [PATCH] t9001: set TERM=dumb to prevent ANSI output

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Christian Couder <chriscool@tuxfamily.org> writes:
> ...
>> This may not work well with colored output.
>> In test-lib.sh line 62 there is :
>>
>> [ "x$TERM" != "xdumb" ] &&
>> 	[ -t 1 ] &&
>> 	tput bold >/dev/null 2>&1 &&
>> 	tput setaf 1 >/dev/null 2>&1 &&
>> 	tput sgr0 >/dev/null 2>&1 &&
>> 	color=t
>
> Ahh.  Sorry about that.

Ahem, third time lucky.

-- >8 --
[PATCH] test-lib: fix TERM to dumb for test repeatability

Dscho noticed that Term::ReadLine (used by send-email) colorized its
output for his TERM settings, inside t9001 tests.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/test-lib.sh |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index c0c5e21..44f5776 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -3,12 +3,16 @@
 # Copyright (c) 2005 Junio C Hamano
 #
 
+# Keep the original TERM for say_color
+ORIGINAL_TERM=$TERM
+
 # For repeatability, reset the environment to known value.
 LANG=C
 LC_ALL=C
 PAGER=cat
 TZ=UTC
-export LANG LC_ALL PAGER TZ
+TERM=dumb
+export LANG LC_ALL PAGER TERM TZ
 EDITOR=:
 VISUAL=:
 unset GIT_EDITOR
@@ -58,12 +62,14 @@ esac
 # This test checks if command xyzzy does the right thing...
 # '
 # . ./test-lib.sh
-
-[ "x$TERM" != "xdumb" ] &&
-	[ -t 1 ] &&
-	tput bold >/dev/null 2>&1 &&
-	tput setaf 1 >/dev/null 2>&1 &&
-	tput sgr0 >/dev/null 2>&1 &&
+[ "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
 
 while test "$#" -ne 0
@@ -91,6 +97,9 @@ done
 
 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 bold; tput setaf 2;; # bold green
@@ -101,6 +110,7 @@ if test -n "$color"; then
 		shift
 		echo "* $*"
 		tput sgr0
+		)
 	}
 else
 	say_color() {
-- 
1.5.4.3.587.g0bdd73

Re: [PATCH] t9001: set TERM=dumb to prevent ANSI output

[Johannes Schindelin]

Hi,

On Thu, 6 Mar 2008, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Christian Couder <chriscool@tuxfamily.org> writes:
> > ...
> >> This may not work well with colored output.
> >> In test-lib.sh line 62 there is :
> >>
> >> [ "x$TERM" != "xdumb" ] &&
> >> 	[ -t 1 ] &&
> >> 	tput bold >/dev/null 2>&1 &&
> >> 	tput setaf 1 >/dev/null 2>&1 &&
> >> 	tput sgr0 >/dev/null 2>&1 &&
> >> 	color=t
> >
> > Ahh.  Sorry about that.
> 
> Ahem, third time lucky.

Well, with this:

>  t/test-lib.sh |   24 +++++++++++++++++-------

I think it is no longer worth it _not_ to special-case it.  IOW I'd prefer 
my original patch for simplicity.

Ciao,
Dscho

Re: [PATCH] t9001: set TERM=dumb to prevent ANSI output

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Thu, 6 Mar 2008, Junio C Hamano wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> > Christian Couder <chriscool@tuxfamily.org> writes:
>> > ...
>> >> This may not work well with colored output.
>> >> In test-lib.sh line 62 there is :
>> >>
>> >> [ "x$TERM" != "xdumb" ] &&
>> >> 	[ -t 1 ] &&
>> >> 	tput bold >/dev/null 2>&1 &&
>> >> 	tput setaf 1 >/dev/null 2>&1 &&
>> >> 	tput sgr0 >/dev/null 2>&1 &&
>> >> 	color=t
>> >
>> > Ahh.  Sorry about that.
>> 
>> Ahem, third time lucky.
>
> Well, with this:
>
>>  t/test-lib.sh |   24 +++++++++++++++++-------
>
> I think it is no longer worth it _not_ to special-case it.  IOW I'd prefer 
> my original patch for simplicity.

Hmm.  Do you still prefer "patch individual" approach, even after
considering longer term maintainability?

Re: [PATCH] t9001: set TERM=dumb to prevent ANSI output

[Johannes Schindelin]

Hi,

On Fri, 7 Mar 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Hi,
> >
> > On Thu, 6 Mar 2008, Junio C Hamano wrote:
> >
> >> Junio C Hamano <gitster@pobox.com> writes:
> >> 
> >> > Christian Couder <chriscool@tuxfamily.org> writes:
> >> > ...
> >> >> This may not work well with colored output.
> >> >> In test-lib.sh line 62 there is :
> >> >>
> >> >> [ "x$TERM" != "xdumb" ] &&
> >> >> 	[ -t 1 ] &&
> >> >> 	tput bold >/dev/null 2>&1 &&
> >> >> 	tput setaf 1 >/dev/null 2>&1 &&
> >> >> 	tput sgr0 >/dev/null 2>&1 &&
> >> >> 	color=t
> >> >
> >> > Ahh.  Sorry about that.
> >> 
> >> Ahem, third time lucky.
> >
> > Well, with this:
> >
> >>  t/test-lib.sh |   24 +++++++++++++++++-------
> >
> > I think it is no longer worth it _not_ to special-case it.  IOW I'd prefer 
> > my original patch for simplicity.
> 
> Hmm.  Do you still prefer "patch individual" approach, even after
> considering longer term maintainability?

Well, I am not the maintainer, and I don't envy you for that.

You are a kick-ass maintainer, so I think whatever you feel is the best 
way to proceed now, is good.

Ciao,
Dscho