git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
To: Junio C Hamano <gitster@pobox.com>
Cc: GIT Mailing-list <git@vger.kernel.org>
Subject: Re: [PATCH 1/1] tests: Allow customization of how say_color() prints
Date: Mon, 17 Dec 2012 22:31:40 +0000	[thread overview]
Message-ID: <50CF9D4C.9080706@ramsay1.demon.co.uk> (raw)
In-Reply-To: <7vobhuqzdc.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index f50f834..9dcf3c1 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -202,6 +202,15 @@ do
>>  	esac
>>  done
>>  
>> +if test -z "$GIT_TEST_PRINT"
>> +then
>> +	GIT_TEST_PRINT="printf %s"
>> +fi
>> +if test -z "$GIT_TEST_PRINT_LN"
>> +then
>> +	GIT_TEST_PRINT_LN="printf %s\n"
>> +fi
>> +
>>  if test -n "$color"
>>  then
>>  	say_color () {
>> @@ -221,7 +230,7 @@ then
>>  			test -n "$quiet" && return;;
>>  		esac
>>  		shift
>> -		printf "%s" "$*"
>> +		$GIT_TEST_PRINT "$*"
>>  		tput sgr0
>>  		echo
>>  		)
>> @@ -230,7 +239,7 @@ else
>>  	say_color() {
>>  		test -z "$1" && test -n "$quiet" && return
>>  		shift
>> -		printf "%s\n" "$*"
>> +		$GIT_TEST_PRINT_LN "$*"
>>  	}
>>  fi
> 
> As you said, this is ugly and also unwieldy in that I do not see an
> easy way for a platform/builder to define something that needs to
> pass a parameter with $IFS in it in these two variables.

Yes, I spent 10 minutes trying to decide if I should send this patch
at all ... (ie how much public humiliation could I take :-D )

> Why does your printf die in the first place???

I really don't know. I'm not sure I will ever know. A couple of years
ago, when I was trying to debug the (harmless) "--color" spew, I found
(via google, etc) numerous accounts of similar problems, with various
workarounds for specific problems. One such account claimed that the
cause of the problem was an official "fix" from Microsoft (as part of
a service pack) which worked just fine on Windows Vista (and later) but
had this known side-effect on XP. Since it fixed the problem it was
intended to fix, even on XP, and the unfortunate "side-effect" on XP
should be quite rare, they decided to apply it on XP anyway. :(

Hmm, on reflection, it would probably be best if you just drop this patch.
I can keep it locally and apply it on top of any branch I want to test.
(Actually, it would be easier to simply revert commit 7bc0911d.)

Sorry for wasting your time.

ATB,
Ramsay Jones

  reply	other threads:[~2012-12-17 22:36 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-15 19:12 [PATCH 1/1] tests: Allow customization of how say_color() prints Ramsay Jones
2012-12-16  6:34 ` Junio C Hamano
2012-12-17 22:31   ` Ramsay Jones [this message]
2012-12-18  1:42     ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=50CF9D4C.9080706@ramsay1.demon.co.uk \
    --to=ramsay@ramsay1.demon.co.uk \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).