Re: [PATCH v5 4/4] commit: Add commit.verbose configuration

[Caleb Thompson <caleb@calebthompson.io>]

[-- Attachment #1: Type: text/plain, Size: 2040 bytes --]

On Mon, Jun 16, 2014 at 01:06:45PM -0700, Junio C Hamano wrote:
> Caleb Thompson <caleb@calebthompson.io> writes:
>
> > On Fri, Jun 13, 2014 at 10:48:55AM -0700, Junio C Hamano wrote:
> >> Caleb Thompson <caleb@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

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]