Re: [PATCH] Fix and improve t7004

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 16, 2007, 21:42 UTC
Message-ID: <7vbq9tyii3.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20071116211148.GA28966@glandium.org>

Mike Hommey <mh@glandium.org> writes:

Show 21 quoted lines

>>> +	( 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"