From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 03 Oct 2010 23:34:26 GMT
Subject: Re: [PATCHv5 16/16] Introduce portable_unset and use it to ensure proper && chaining
Message-ID: <7vzkuudf5p.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20101003145122.GF17084@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Elijah Newren wrote:
>
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -268,6 +268,10 @@ remove_cr () {
>>  	tr '\015' Q | sed -e 's/Q$//'
>>  }
>>  
>> +portable_unset () {
>> +	unset $* || true
>> +}
>
> I think this should read
>
> 	portable_unset () {
> 		unset "$@"
> 		return 0
> 	}
>
> (or || true).  That is, if I try
>
> 	portable_unset "foo bar"
>
> then on platforms where an envvar named "foo bar" is allowed,
> this should unset it, no?

Yes, or closer to Elijah's original:

	unset "$@" || true

I do not think calling this "portable_" makes much sense, though.  Maybe
depending on the exit status of "unset" may be unportable, but the wrapper
does not make unset portable.  It just picks one behaviour we happen to
like among the possible two (one being "unset always succeeds" vs the
other being "unset errors out if you feed an unset variable").

I have no objection against calling it "sane_unset", though.  It makes it
crystal clear that the semantics wrapper implements is what we picked
because we like (i.e. deem the sanest among alternatives).