From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 16 Nov 2007 21:42:44 GMT
Subject: Re: [PATCH] Fix and improve t7004
Message-ID: <7vbq9tyii3.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20071116211148.GA28966@glandium.org>

Mike Hommey <mh@glandium.org> writes:

>>> +	( read empty ;
>>> +	  [ "$empty" ] && exit 1 ;
>>
>> What is this meant to do?  Did you mean [ -n "$empty" ] ?
>
> Replacing with [ -n "$empty" ] would not work properly, except if you
> replace the following ; with &&. Does that really make a readability
> difference ?
>
>>> +	  ! grep -ve "^#" > /dev/null 2>&1 ) < actual
>>
>> The double negation is harder to read.  May I suggest something along these 
>> lines (which seems more readable to me):
>>   while read line; do
>>     case $line in #(
>>       '#'*) ;; # Accept comments (
>>       *) exit 1;;
>>     esac
>>   done
>
> I'm not really convinced. What do other people have to say ?

As shell "read" loses information (a backslash sequence is
interpreted, and trailing whitespaces are stripped and not
assigned to "line" above), it is not such a good vehicle if you
want to make a reasonably strict test on top of.  Some shells
do not implement "read -r" either, so it is also a portability
hassle.

Perhaps...

	# check the first line --- should be empty
	first=$(sed -e 1q <actual) &&
        test -z "$first" &&
	# remove commented lines from the remainder -- should be empty
        rest=$(sed -e 1d -e '/^#/d' <actual) &&
        test -z "$rest"