On Fri, May 12, 2017 at 6:48 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> Add exhaustive tests for how the different grep.patternType options & >> the corresponding command-line options affect git-log. >> ... >> The patterns being passed to fixed/basic/extended/PCRE are carefully >> crafted to return the wrong thing if the grep engine were to pick any >> other matching method than the one it's told to use. > > That is good; it may be even more helpful to the readers if they are > given a brief in-code comment. I had to scratch head while reading > them. > >> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> >> --- >> t/t4202-log.sh | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 76 insertions(+), 1 deletion(-) >> >> diff --git a/t/t4202-log.sh b/t/t4202-log.sh >> index f577990716..6d1411abea 100755 >> --- a/t/t4202-log.sh >> +++ b/t/t4202-log.sh >> @@ -262,7 +262,23 @@ test_expect_success 'log --grep -i' ' >> >> test_expect_success 'log -F -E --grep=<ere> uses ere' ' >> echo second >expect && >> - git log -1 --pretty="tformat:%s" -F -E --grep=s.c.nd >actual && > # BRE would need \(s\) to do the same. >> + git log -1 --pretty="tformat:%s" -F -E --grep="(s).c.nd" >actual && >> + test_cmp expect actual >> +' >> + >> +test_expect_success PCRE 'log -F -E --perl-regexp --grep=<pcre> uses PCRE' ' >> + test_when_finished "rm -rf num_commits" && >> + git init num_commits && >> + ( >> + cd num_commits && >> + test_commit 1d && >> + test_commit 2e >> + ) && >> + echo 2e >expect && > # In PCRE \d in [\d] is like saying "0-9", and match '2' in 2e >> + git -C num_commits log -1 --pretty="tformat:%s" -F -E --perl-regexp --grep="[\d]" >actual && >> + test_cmp expect actual && >> + echo 1d >expect && > # In ERE [\d] is bs or letter 'd' and would not match '2' in '2e' > # but does match 'd' in '1d' >> + git -C num_commits log -1 --pretty="tformat:%s" -F -E --grep="[\d]" >actual && >> test_cmp expect actual >> ' >> >> @@ -280,6 +296,65 @@ test_expect_success 'log with grep.patternType configuration and command line' ' >> test_cmp expect actual >> ' >> >> +test_expect_success 'log with various grep.patternType configurations & command-lines' ' >> + git init pattern-type && >> + ( >> + cd pattern-type && >> + test_commit 1 file A && >> + test_commit "(1|2)" file B && >> + >> + echo "(1|2)" >expect.fixed && >> + cp expect.fixed expect.basic && >> + cp expect.fixed expect.extended && >> + cp expect.fixed expect.perl && >> + > # Fixed finds these literally >> + git -c grep.patternType=fixed log --pretty=tformat:%s \ >> + --grep="(1|2)" >actual.fixed && > # BRE matches (, |, and ) literally >> + git -c grep.patternType=basic log --pretty=tformat:%s \ >> + --grep="(.|.)" >actual.basic && > # ERE needs | quoted with bs to match | literally >> + git -c grep.patternType=extended log --pretty=tformat:%s \ >> + --grep="\|2" >actual.extended && > > If we use BRE by mistake, wouldn't this pattern actually find > "(1|2)" anyway, without skipping it and show "1 file A" instead? It'll find (1|2) but also 1, i.e.: $ (echo 1; echo "(1|2)") >/tmp/f; for t in G E; do echo $t: && grep -$t '\|2' /tmp/f | sed 's/^/ /'; done G: 1 (1|2) E: (1|2) So the test will fail under basic. I'll add comments about this & the other things you suggested. >> + if test_have_prereq PCRE >> + then >> + git -c grep.patternType=perl log --pretty=tformat:%s \ >> + --grep="[\d]\|" >actual.perl > # Only PCRE would match [\d]\| with "(1|2)" due to [\d] >> + fi && >> + test_cmp expect.fixed actual.fixed && >> + test_cmp expect.basic actual.basic && >> + test_cmp expect.extended actual.extended && >> + if test_have_prereq PCRE >> + then >> + test_cmp expect.perl actual.perl >> + fi && > > It could be just a style thing, but I would actually find it easier > to follow if the above two "only with PCRE" tests, i.e. running test > and taking its output in actual.perl and comparing it with > expect.perl, were inside a single "if test_have_prereq PCRE" block. Sure, will fix.