Re: [PATCH v3 2/2] diff: add -I<regex> that ignores matching changes

> Michał Kępień <michal@isc.org> writes:
> 
> > +test_expect_success 'diff -I<regex>' '
> > +	test_seq 50 >I.txt &&
> > +	sed -e "s/13/ten and three/" -e "/7\$/d" <I.txt >J.txt &&
> > +	echo >>J.txt &&
> > +
> > +	test_expect_code 1 git diff --no-index --ignore-blank-lines -I"ten.*e" -I"^[124-9]" I.txt J.txt >actual &&
> 
> Please wrap an overly long line like this oned, perhaps like:
> 
> 	test_expect_code 1 git diff --no-index --ignore-blank-lines \
> 		-I"ten.*e" -I"^[124-9]" I.txt J.txt >actual &&

> > +	test_expect_code 1 git diff --stat --no-index --ignore-blank-lines -I"ten.*e" -I"^[124-9]" I.txt J.txt >actual &&
> 
> > +	test_expect_code 129 git diff --no-index --ignore-matching-lines="^[124-9]" --ignore-matching-lines="^[124-9" I.txt J.txt >output 2>&1 &&
> 
> Cramming unrelated tests into a single one made me puzzled, staring
> at this line for longer than necessary before realizing that this is
> an attempt to catch a malformed regexp.  If this were in a separate
> test with its own title, e.g.
> 
> (...)
> 		
> It would have been much easier to follow.
> 
> It also is curious that it only tests --no-index, which is a bolted
> on feature that may not exercise the codepath the users depend on
> working correctly.  If this were tested with "git log -p" for a few
> commits, the early destruction of regexp may have been caught,
> especially with ASAN build.