git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH/RFC] test-lib: add support for colors without tput
@ 2012-09-14 16:41 Erik Faye-Lund
  2012-09-14 16:54 ` Erik Faye-Lund
  2012-09-14 17:44 ` Jeff King
  0 siblings, 2 replies; 17+ messages in thread
From: Erik Faye-Lund @ 2012-09-14 16:41 UTC (permalink / raw)
  To: git; +Cc: msysgit

For platforms that does not have tput we can still perform coloring
by manually emitting the ANSI control codes. If tput is missing from
$PATH, install a replacement function.

The exact strings has been dumped from a machine that has tput, by
piping the output of tput through 'od -c -An'.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

I got slightly annoyed that we didn't get colored output from the
tests on Windows, so I decided to fix it.

Hopefully other platforms can benefit from this as well.

I'm not super happy with the condition to enable it. I considered
an environment variable as well, but decided against it because
"make -C t" from the root does not seem to pick up environment
variables configured in the main Makefile.

Thoughts?

 t/test-lib.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 78c4286..7d1b34b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
 # This test checks if command xyzzy does the right thing...
 # '
 # . ./test-lib.sh
+
+if ! which tput > /dev/null ; then
+	tput () {
+		case "$1" in
+		bold)
+			echo -ne "\033[1m" ;;
+		setaf)
+			echo -ne "\033[0;3$2m" ;;
+		sgr0)
+			echo -ne "\033(\033[m" ;;
+		esac
+	}
+fi
+
 [ "x$ORIGINAL_TERM" != "xdumb" ] && (
 		TERM=$ORIGINAL_TERM &&
 		export TERM &&
-- 
1.7.11.msysgit.0.5.g0225efe.dirty

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 16:41 [PATCH/RFC] test-lib: add support for colors without tput Erik Faye-Lund
@ 2012-09-14 16:54 ` Erik Faye-Lund
  2012-09-14 16:58   ` Erik Faye-Lund
  2012-09-14 17:30   ` Junio C Hamano
  2012-09-14 17:44 ` Jeff King
  1 sibling, 2 replies; 17+ messages in thread
From: Erik Faye-Lund @ 2012-09-14 16:54 UTC (permalink / raw)
  To: git; +Cc: msysgit

On Fri, Sep 14, 2012 at 6:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 78c4286..7d1b34b 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>  # This test checks if command xyzzy does the right thing...
>  # '
>  # . ./test-lib.sh
> +
> +if ! which tput > /dev/null ; then
> +       tput () {
> +               case "$1" in
> +               bold)
> +                       echo -ne "\033[1m" ;;
> +               setaf)
> +                       echo -ne "\033[0;3$2m" ;;
> +               sgr0)
> +                       echo -ne "\033(\033[m" ;;

I should of course have checked this earlier, but I find now that
"echo -ne" isn't portable. So perhaps this on top?

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7d1b34b..91a1d7b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,11 +134,11 @@ if ! which tput > /dev/null ; then
 	tput () {
 		case "$1" in
 		bold)
-			echo -ne "\033[1m" ;;
+			printf "%b" "\033[1m" ;;
 		setaf)
-			echo -ne "\033[0;3$2m" ;;
+			printf "%b" "\033[0;3$2m" ;;
 		sgr0)
-			echo -ne "\033(\033[m" ;;
+			printf "%b" "\033(\033[m" ;;
 		esac
 	}
 fi

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 16:54 ` Erik Faye-Lund
@ 2012-09-14 16:58   ` Erik Faye-Lund
  2012-09-14 17:08     ` Elia Pinto
  2012-09-14 17:28     ` Johannes Sixt
  2012-09-14 17:30   ` Junio C Hamano
  1 sibling, 2 replies; 17+ messages in thread
From: Erik Faye-Lund @ 2012-09-14 16:58 UTC (permalink / raw)
  To: git; +Cc: msysgit

On Fri, Sep 14, 2012 at 6:54 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Fri, Sep 14, 2012 at 6:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 78c4286..7d1b34b 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>>  # This test checks if command xyzzy does the right thing...
>>  # '
>>  # . ./test-lib.sh
>> +
>> +if ! which tput > /dev/null ; then
>> +       tput () {
>> +               case "$1" in
>> +               bold)
>> +                       echo -ne "\033[1m" ;;
>> +               setaf)
>> +                       echo -ne "\033[0;3$2m" ;;
>> +               sgr0)
>> +                       echo -ne "\033(\033[m" ;;
>
> I should of course have checked this earlier, but I find now that
> "echo -ne" isn't portable. So perhaps this on top?
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 7d1b34b..91a1d7b 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -134,11 +134,11 @@ if ! which tput > /dev/null ; then
>         tput () {
>                 case "$1" in
>                 bold)
> -                       echo -ne "\033[1m" ;;
> +                       printf "%b" "\033[1m" ;;
>                 setaf)
> -                       echo -ne "\033[0;3$2m" ;;
> +                       printf "%b" "\033[0;3$2m" ;;
>                 sgr0)
> -                       echo -ne "\033(\033[m" ;;
> +                       printf "%b" "\033(\033[m" ;;
>                 esac
>         }
>  fi

And again, I'm stupid for not reading documentation properly; octal
escaped strings in the format string should work (and does on my
systems), so this is sufficient:

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7d1b34b..2a6149e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,11 +134,11 @@ if ! which tput > /dev/null ; then
 	tput () {
 		case "$1" in
 		bold)
-			echo -ne "\033[1m" ;;
+			printf "\033[1m" ;;
 		setaf)
-			echo -ne "\033[0;3$2m" ;;
+			printf "\033[0;3$2m" ;;
 		sgr0)
-			echo -ne "\033(\033[m" ;;
+			printf "\033(\033[m" ;;
 		esac
 	}
 fi

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 16:58   ` Erik Faye-Lund
@ 2012-09-14 17:08     ` Elia Pinto
  2012-09-14 17:11       ` Erik Faye-Lund
  2012-09-14 17:12       ` Elia Pinto
  2012-09-14 17:28     ` Johannes Sixt
  1 sibling, 2 replies; 17+ messages in thread
From: Elia Pinto @ 2012-09-14 17:08 UTC (permalink / raw)
  To: kusmabite; +Cc: git, msysgit

2012/9/14 Erik Faye-Lund <kusmabite@gmail.com>:
> On Fri, Sep 14, 2012 at 6:54 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> On Fri, Sep 14, 2012 at 6:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>>> index 78c4286..7d1b34b 100644
>>> --- a/t/test-lib.sh
>>> +++ b/t/test-lib.sh
>>> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>>>  # This test checks if command xyzzy does the right thing...
>>>  # '
>>>  # . ./test-lib.sh
>>> +
Nice. But this setting should be check that we have a terminal first isn't ?
Some test like this before

test "X$$TERM" != Xdumb \
&&  test -t 1 2>/dev/null  \
&& ....

or the inverse logic. This is what automake  and popt autogen.sh does.

Best Regards

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 17:08     ` Elia Pinto
@ 2012-09-14 17:11       ` Erik Faye-Lund
  2012-09-14 17:12       ` Elia Pinto
  1 sibling, 0 replies; 17+ messages in thread
From: Erik Faye-Lund @ 2012-09-14 17:11 UTC (permalink / raw)
  To: Elia Pinto; +Cc: git, msysgit

On Fri, Sep 14, 2012 at 7:08 PM, Elia Pinto <gitter.spiros@gmail.com> wrote:
> 2012/9/14 Erik Faye-Lund <kusmabite@gmail.com>:
>> On Fri, Sep 14, 2012 at 6:54 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> On Fri, Sep 14, 2012 at 6:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>>>> index 78c4286..7d1b34b 100644
>>>> --- a/t/test-lib.sh
>>>> +++ b/t/test-lib.sh
>>>> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>>>>  # This test checks if command xyzzy does the right thing...
>>>>  # '
>>>>  # . ./test-lib.sh
>>>> +
> Nice. But this setting should be check that we have a terminal first isn't ?
> Some test like this before
>
> test "X$$TERM" != Xdumb \
> &&  test -t 1 2>/dev/null  \
> && ....
>
> or the inverse logic. This is what automake  and popt autogen.sh does.

There's already such a check a few lines further down, and tput isn't
used in such cases.

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 17:08     ` Elia Pinto
  2012-09-14 17:11       ` Erik Faye-Lund
@ 2012-09-14 17:12       ` Elia Pinto
  2012-09-14 17:16         ` Erik Faye-Lund
  1 sibling, 1 reply; 17+ messages in thread
From: Elia Pinto @ 2012-09-14 17:12 UTC (permalink / raw)
  To: kusmabite; +Cc: git, msysgit

2012/9/14 Elia Pinto <gitter.spiros@gmail.com>:
> 2012/9/14 Erik Faye-Lund <kusmabite@gmail.com>:
>> On Fri, Sep 14, 2012 at 6:54 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> On Fri, Sep 14, 2012 at 6:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>>>> index 78c4286..7d1b34b 100644
>>>> --- a/t/test-lib.sh
>>>> +++ b/t/test-lib.sh
>>>> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>>>>  # This test checks if command xyzzy does the right thing...
>>>>  # '
>>>>  # . ./test-lib.sh
>>>> +
> Nice. But this setting should be check that we have a terminal first isn't ?
> Some test like this before
>
> test "X$$TERM" != Xdumb \
> &&  test -t 1 2>/dev/null  \
> && ....
and in reality this echo use is not portable.
http://ftp.gnu.org/old-gnu/Manuals/autoconf-2.53/html_node/Limitations-of-Builtins.html

In popt 1_17 autogen.sh does

red=; grn=; lgn=; blu=; std=;
test "X$$TERM" != Xdumb \
&&  test -t 1 2>/dev/null  \
&& { \
  red='^[[0;31m'; \
  grn='^[[0;32m'; \
  lgn='^[[1;32m'; \
  blu='^[[1;34m'; \
  std='^[[m'; \
}

and

Die()    {
        color="$red"
        echo "${color}${_PROGNAME}: Error: $@${std}" >&2
        exit 1
}

Die "message here"


>
> or the inverse logic. This is what automake  and popt autogen.sh does.
>
> Best Regards

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 17:12       ` Elia Pinto
@ 2012-09-14 17:16         ` Erik Faye-Lund
  0 siblings, 0 replies; 17+ messages in thread
From: Erik Faye-Lund @ 2012-09-14 17:16 UTC (permalink / raw)
  To: Elia Pinto; +Cc: git, msysgit

On Fri, Sep 14, 2012 at 7:12 PM, Elia Pinto <gitter.spiros@gmail.com> wrote:
> 2012/9/14 Elia Pinto <gitter.spiros@gmail.com>:
>> 2012/9/14 Erik Faye-Lund <kusmabite@gmail.com>:
>>> On Fri, Sep 14, 2012 at 6:54 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>>> On Fri, Sep 14, 2012 at 6:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>>>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>>>>> index 78c4286..7d1b34b 100644
>>>>> --- a/t/test-lib.sh
>>>>> +++ b/t/test-lib.sh
>>>>> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>>>>>  # This test checks if command xyzzy does the right thing...
>>>>>  # '
>>>>>  # . ./test-lib.sh
>>>>> +
>> Nice. But this setting should be check that we have a terminal first isn't ?
>> Some test like this before
>>
>> test "X$$TERM" != Xdumb \
>> &&  test -t 1 2>/dev/null  \
>> && ....
> and in reality this echo use is not portable.

Yeah; I posted a couple of follow-up mails earlier where I had noticed
it and changed to printf instead. It seems the testsuite is already
using it, so it's probably portable.

Thanks a lot for the extra set of eyes :)

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 16:58   ` Erik Faye-Lund
  2012-09-14 17:08     ` Elia Pinto
@ 2012-09-14 17:28     ` Johannes Sixt
  2012-09-14 17:31       ` Erik Faye-Lund
                         ` (2 more replies)
  1 sibling, 3 replies; 17+ messages in thread
From: Johannes Sixt @ 2012-09-14 17:28 UTC (permalink / raw)
  To: kusmabite; +Cc: git, msysgit

Am 14.09.2012 18:58, schrieb Erik Faye-Lund:
>  	tput () {
>  		case "$1" in
>  		bold)
> -			echo -ne "\033[1m" ;;
> +			printf "\033[1m" ;;
>  		setaf)
> -			echo -ne "\033[0;3$2m" ;;
> +			printf "\033[0;3$2m" ;;

This should be
			printf '\033[0;3%sm' "$2" ;;

>  		sgr0)
> -			echo -ne "\033(\033[m" ;;
> +			printf "\033(\033[m" ;;
>  		esac
>  	}
>  fi

Did you test this only in rxvt or in CMD as well? (I hadn't time to
test, yet, so I'm asking :-)

-- Hannes

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 16:54 ` Erik Faye-Lund
  2012-09-14 16:58   ` Erik Faye-Lund
@ 2012-09-14 17:30   ` Junio C Hamano
  2012-09-14 17:42     ` Erik Faye-Lund
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2012-09-14 17:30 UTC (permalink / raw)
  To: kusmabite; +Cc: git, msysgit

Erik Faye-Lund <kusmabite@gmail.com> writes:

> On Fri, Sep 14, 2012 at 6:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 78c4286..7d1b34b 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>>  # This test checks if command xyzzy does the right thing...
>>  # '
>>  # . ./test-lib.sh
>> +
>> +if ! which tput > /dev/null ; then
>> +       tput () {
>> +               case "$1" in
>> +               bold)
>> +                       echo -ne "\033[1m" ;;
>> +               setaf)
>> +                       echo -ne "\033[0;3$2m" ;;
>> +               sgr0)
>> +                       echo -ne "\033(\033[m" ;;
>
> I should of course have checked this earlier, but I find now that
> "echo -ne" isn't portable.

Neither is which, no?

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 17:28     ` Johannes Sixt
@ 2012-09-14 17:31       ` Erik Faye-Lund
  2012-09-14 18:11       ` Erik Faye-Lund
  2012-09-17 17:39       ` Johannes Sixt
  2 siblings, 0 replies; 17+ messages in thread
From: Erik Faye-Lund @ 2012-09-14 17:31 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, msysgit

On Fri, Sep 14, 2012 at 7:28 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 14.09.2012 18:58, schrieb Erik Faye-Lund:
>>       tput () {
>>               case "$1" in
>>               bold)
>> -                     echo -ne "\033[1m" ;;
>> +                     printf "\033[1m" ;;
>>               setaf)
>> -                     echo -ne "\033[0;3$2m" ;;
>> +                     printf "\033[0;3$2m" ;;
>
> This should be
>                         printf '\033[0;3%sm' "$2" ;;
>

That's probably a good idea, yeah.

>>               sgr0)
>> -                     echo -ne "\033(\033[m" ;;
>> +                     printf "\033(\033[m" ;;
>>               esac
>>       }
>>  fi
>
> Did you test this only in rxvt or in CMD as well? (I hadn't time to
> test, yet, so I'm asking :-)

I don't have rxvt installed, but it works for me in CMD also.

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 17:30   ` Junio C Hamano
@ 2012-09-14 17:42     ` Erik Faye-Lund
  2012-09-14 18:03       ` Junio C Hamano
  0 siblings, 1 reply; 17+ messages in thread
From: Erik Faye-Lund @ 2012-09-14 17:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, msysgit

On Fri, Sep 14, 2012 at 7:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> On Fri, Sep 14, 2012 at 6:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>>> index 78c4286..7d1b34b 100644
>>> --- a/t/test-lib.sh
>>> +++ b/t/test-lib.sh
>>> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>>>  # This test checks if command xyzzy does the right thing...
>>>  # '
>>>  # . ./test-lib.sh
>>> +
>>> +if ! which tput > /dev/null ; then
>>> +       tput () {
>>> +               case "$1" in
>>> +               bold)
>>> +                       echo -ne "\033[1m" ;;
>>> +               setaf)
>>> +                       echo -ne "\033[0;3$2m" ;;
>>> +               sgr0)
>>> +                       echo -ne "\033(\033[m" ;;
>>
>> I should of course have checked this earlier, but I find now that
>> "echo -ne" isn't portable.
>
> Neither is which, no?

Oooh, right. Thanks for noticing. So I guess I should try to run it
instead. From the POSIX spec, I can't find a way of running it that
guarantees a return-code of 0 without clobbering the console somehow.

Perhaps the best thing is pass no operands, and check for $? == 127 instead?

Something like this?

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a939e19..1433cb3 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -130,7 +130,8 @@ export _x05 _x40 _z40 LF
 # '
 # . ./test-lib.sh

-if ! which tput > /dev/null ; then
+tput > /dev/null
+if test $? -eq 127 ; then
 	tput () {
 		case "$1" in
 		bold)

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 16:41 [PATCH/RFC] test-lib: add support for colors without tput Erik Faye-Lund
  2012-09-14 16:54 ` Erik Faye-Lund
@ 2012-09-14 17:44 ` Jeff King
  2012-09-14 17:52   ` Erik Faye-Lund
  1 sibling, 1 reply; 17+ messages in thread
From: Jeff King @ 2012-09-14 17:44 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: git, msysgit

On Fri, Sep 14, 2012 at 06:41:45PM +0200, Erik Faye-Lund wrote:

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 78c4286..7d1b34b 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>  # This test checks if command xyzzy does the right thing...
>  # '
>  # . ./test-lib.sh
> +
> +if ! which tput > /dev/null ; then

Testing the return value of "which" is not portable (I know, it's
insane; SunOS is the common offender). Use "type" instead.

-Peff

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 17:44 ` Jeff King
@ 2012-09-14 17:52   ` Erik Faye-Lund
  0 siblings, 0 replies; 17+ messages in thread
From: Erik Faye-Lund @ 2012-09-14 17:52 UTC (permalink / raw)
  To: Jeff King; +Cc: git, msysgit

On Fri, Sep 14, 2012 at 7:44 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Sep 14, 2012 at 06:41:45PM +0200, Erik Faye-Lund wrote:
>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 78c4286..7d1b34b 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -129,6 +129,20 @@ export _x05 _x40 _z40 LF
>>  # This test checks if command xyzzy does the right thing...
>>  # '
>>  # . ./test-lib.sh
>> +
>> +if ! which tput > /dev/null ; then
>
> Testing the return value of "which" is not portable (I know, it's
> insane; SunOS is the common offender). Use "type" instead.

Junio already noticed it, and I suggested a fix that involved running
it. However, I like your fix much better, thanks :)

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 17:42     ` Erik Faye-Lund
@ 2012-09-14 18:03       ` Junio C Hamano
  0 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2012-09-14 18:03 UTC (permalink / raw)
  To: kusmabite; +Cc: git, msysgit

Erik Faye-Lund <kusmabite@gmail.com> writes:

>> Neither is which, no?
>
> Oooh, right. Thanks for noticing. So I guess I should try to run it
> instead. From the  POSIX spec,...

If you assume POSIX.1, there is "type".

    $ type frotz ; echo $?
    frotz is /usr/games/frotz
    0
    $ type frobnitz ; echo $?
    bash: type: frobnitz: not found
    1

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 17:28     ` Johannes Sixt
  2012-09-14 17:31       ` Erik Faye-Lund
@ 2012-09-14 18:11       ` Erik Faye-Lund
  2012-09-14 19:15         ` Johannes Sixt
  2012-09-17 17:39       ` Johannes Sixt
  2 siblings, 1 reply; 17+ messages in thread
From: Erik Faye-Lund @ 2012-09-14 18:11 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, msysgit

On Fri, Sep 14, 2012 at 7:28 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 14.09.2012 18:58, schrieb Erik Faye-Lund:
>>       tput () {
>>               case "$1" in
>>               bold)
>> -                     echo -ne "\033[1m" ;;
>> +                     printf "\033[1m" ;;
>>               setaf)
>> -                     echo -ne "\033[0;3$2m" ;;
>> +                     printf "\033[0;3$2m" ;;
>
> This should be
>                         printf '\033[0;3%sm' "$2" ;;

Is there a reason for %s rather than %d? It seem it only takes
integers, and with %d at least we'd get an error message if someone
started doing something else...

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 18:11       ` Erik Faye-Lund
@ 2012-09-14 19:15         ` Johannes Sixt
  0 siblings, 0 replies; 17+ messages in thread
From: Johannes Sixt @ 2012-09-14 19:15 UTC (permalink / raw)
  To: kusmabite; +Cc: git, msysgit

Am 14.09.2012 20:11, schrieb Erik Faye-Lund:
> On Fri, Sep 14, 2012 at 7:28 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>>                         printf '\033[0;3%sm' "$2" ;;
> 
> Is there a reason for %s rather than %d? It seem it only takes
> integers,..

No reason. I just mechanically converted your original expression. But
there is no reason for my conversion, either, if it can be more or less
guaranteed that no arbitrary strings are passed in $2.

-- Hannes

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC] test-lib: add support for colors without tput
  2012-09-14 17:28     ` Johannes Sixt
  2012-09-14 17:31       ` Erik Faye-Lund
  2012-09-14 18:11       ` Erik Faye-Lund
@ 2012-09-17 17:39       ` Johannes Sixt
  2 siblings, 0 replies; 17+ messages in thread
From: Johannes Sixt @ 2012-09-17 17:39 UTC (permalink / raw)
  To: kusmabite; +Cc: git, msysgit

Am 14.09.2012 19:28, schrieb Johannes Sixt:
> Am 14.09.2012 18:58, schrieb Erik Faye-Lund:
>>  	tput () {
>>  		case "$1" in
>>  		bold)
>> -			echo -ne "\033[1m" ;;
>> +			printf "\033[1m" ;;
>>  		setaf)
>> -			echo -ne "\033[0;3$2m" ;;
>> +			printf "\033[0;3$2m" ;;
> 
> This should be
> 			printf '\033[0;3%sm' "$2" ;;
> 
>>  		sgr0)
>> -			echo -ne "\033(\033[m" ;;
>> +			printf "\033(\033[m" ;;
>>  		esac
>>  	}
>>  fi
> 
> Did you test this only in rxvt or in CMD as well? (I hadn't time to
> test, yet, so I'm asking :-)

I tested your patch with this fixup, and it works for me (in CMD).

-- Hannes

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2012-09-17 17:39 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-14 16:41 [PATCH/RFC] test-lib: add support for colors without tput Erik Faye-Lund
2012-09-14 16:54 ` Erik Faye-Lund
2012-09-14 16:58   ` Erik Faye-Lund
2012-09-14 17:08     ` Elia Pinto
2012-09-14 17:11       ` Erik Faye-Lund
2012-09-14 17:12       ` Elia Pinto
2012-09-14 17:16         ` Erik Faye-Lund
2012-09-14 17:28     ` Johannes Sixt
2012-09-14 17:31       ` Erik Faye-Lund
2012-09-14 18:11       ` Erik Faye-Lund
2012-09-14 19:15         ` Johannes Sixt
2012-09-17 17:39       ` Johannes Sixt
2012-09-14 17:30   ` Junio C Hamano
2012-09-14 17:42     ` Erik Faye-Lund
2012-09-14 18:03       ` Junio C Hamano
2012-09-14 17:44 ` Jeff King
2012-09-14 17:52   ` Erik Faye-Lund

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