On Mon, Jun 16, 2014 at 01:06:45PM -0700, Junio C Hamano wrote: > Caleb Thompson <ca...@calebthompson.io> writes: > > > On Fri, Jun 13, 2014 at 10:48:55AM -0700, Junio C Hamano wrote: > >> Caleb Thompson <ca...@calebthompson.io> writes: > >> > >> > diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh > >> > index 35a4d06..402d6a1 100755 > >> > --- a/t/t7507-commit-verbose.sh > >> > +++ b/t/t7507-commit-verbose.sh > >> > @@ -7,6 +7,10 @@ write_script check-for-diff <<-'EOF' > >> > exec grep '^diff --git' "$1" > >> > EOF > >> > > >> > +write_script check-for-no-diff <<-'EOF' > >> > + exec grep -v '^diff --git' "$1" > >> > +EOF > >> > >> This lets grep show all lines that are not "diff --git" in the > >> input, and as usual grep exits success if it has any line in the > >> output. > >> > >> $ grep -v '^diff --git' <<\EOF ; echo $? > >> diff --git > >> a > >> EOF > >> a > >> 0 > >> $ exit > >> > >> What are we testing, exactly? > > > > Good catch. It worked when I switched check-for-diff from > > check-for-no-diff, but I didn't try to make check-for-no-diff fail > > independently, so I apologize. > > No need to apologize at all. None of us (including this reviewer) > is perfect and that is why we review patches by each other. > > > This version removes the the beginning of a line starting with > > "diff --git" from the string,... > > Again, what are we testing, exactly? > > We do not want to see "^diff --git" in the output file, in other > words, we want to make sure "^diff --git" does not appear in the > output. > > So > > write_script check-for-no-diff <<-\EOF > ! grep '^diff --git' "$@" > EOF > > should be the most natural way to express what we are testing, no?
I did consider that. The reason I didn't propose that is that it doesn't catch the unlikely case that the $1 only contains a "diff --git" line or that $1 is empty. Those are rather unreasonable concerns, so I'm happy to use the much more readable version as you propose. Caleb Thompson
pgpuRfoSf3aXR.pgp
Description: PGP signature
