From: Junio C Hamano <gitster@pobox.com>
To: Mike Hommey <mh@glandium.org>
Cc: Benoit Sigoure <tsuna@lrde.epita.fr>, git@vger.kernel.org
Subject: Re: [PATCH] Fix and improve t7004
Date: Fri, 16 Nov 2007 13:42:44 -0800 [thread overview]
Message-ID: <7vbq9tyii3.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20071116211148.GA28966@glandium.org> (Mike Hommey's message of "Fri, 16 Nov 2007 22:11:48 +0100")
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"
next prev parent reply other threads:[~2007-11-16 21:43 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-15 13:38 [BUG] t7004 (master) busted on Leopard Wincent Colaiuta
2007-11-15 14:37 ` Johannes Schindelin
2007-11-15 14:48 ` David Kastrup
2007-11-15 15:12 ` Wincent Colaiuta
2007-11-15 15:16 ` [PATCH] Fix git-tag test breakage caused by broken sed " Wincent Colaiuta
2007-11-15 15:47 ` [PATCH v2] " Wincent Colaiuta
[not found] ` <7v4pfm3h6f.fsf@gitster.siamese.dyndns.org>
2007-11-16 13:45 ` Wincent Colaiuta
2007-11-16 13:48 ` Wincent Colaiuta
2007-11-16 16:59 ` Mike Hommey
2007-11-16 17:25 ` Wincent Colaiuta
2007-11-16 17:26 ` [PATCH] Fix t7004 which fails with retarded sed Mike Hommey
2007-11-16 17:58 ` Benoit Sigoure
2007-11-16 19:15 ` Mike Hommey
2007-11-16 18:02 ` Wincent Colaiuta
2007-11-16 18:26 ` Junio C Hamano
2007-11-16 19:17 ` Mike Hommey
2007-11-16 19:36 ` Junio C Hamano
2007-11-16 20:28 ` [PATCH] Fix and improve t7004 Mike Hommey
2007-11-16 21:04 ` Benoit Sigoure
2007-11-16 21:11 ` Mike Hommey
2007-11-16 21:31 ` Benoit Sigoure
2007-11-16 21:35 ` Mike Hommey
2007-11-16 21:47 ` Benoit Sigoure
2007-11-16 21:42 ` Junio C Hamano [this message]
2007-11-16 22:02 ` Mike Hommey
2007-11-17 8:55 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vbq9tyii3.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=mh@glandium.org \
--cc=tsuna@lrde.epita.fr \
/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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.