Re: [PATCHv5 16/16] Introduce portable_unset and use it to ensure proper && chaining

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 3, 2010, 23:34 UTC
Message-ID: <7vzkuudf5p.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20101003145122.GF17084@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 25 quoted lines

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