{"thread":{"id":"5140","subject":"[RFC/PATCH] Fix \"grep -w\"","startedAt":"2006-08-05T05:16:42Z","lastAt":"2006-08-06T08:39:37Z","messageCount":4,"participants":["Junio C Hamano","Morten Welinder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"24794","messageId":"7vwt9ng3t1.fsf@assigned-by-dhcp.cox.net","threadId":"5140","inReplyTo":null,"subject":"[RFC/PATCH] Fix \"grep -w\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-05T05:16:42Z","receivedAt":"2006-08-05T05:16:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We used to find the first match of the pattern and then if the\nmatch is not for the entire word, declared that the whole line\ndoes not match.\n\nBut that is wrong.  The command \"git grep -w -e mmap\" should\nfind that a line \"foo_mmap bar mmap baz\" matches, by tring the\nsecond instance of pattern \"mmap\" on the same line.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n builtin-grep.c  |   10 ++++++++++\n t/t7002-grep.sh |   45 +++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 55 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 69b7c48..b5feda4 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -412,6 +412,7 @@ static int match_one_pattern(struct grep\n \tint hit = 0;\n \tregmatch_t pmatch[10];\n \n+ again:\n \tif (!opt->fixed) {\n \t\tregex_t *exp = &p->regexp;\n \t\thit = !regexec(exp, bol, ARRAY_SIZE(pmatch),\n@@ -438,6 +439,15 @@ static int match_one_pattern(struct grep\n \t\tif (pmatch[0].rm_eo != (eol-bol) &&\n \t\t    word_char(bol[pmatch[0].rm_eo]))\n \t\t\thit = 0;\n+\n+\t\tif (!hit && pmatch[0].rm_eo + bol < eol) {\n+\t\t\t/* there could be more than one match on the\n+\t\t\t * line, and the first match might not be\n+\t\t\t * strict word match.  But later ones could be!\n+\t\t\t */\n+\t\t\tbol += pmatch[0].rm_eo;\n+\t\t\tgoto again;\n+\t\t}\n \t}\n \treturn hit;\n }\ndiff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\nnew file mode 100755\nindex 0000000..0a0e302\n--- /dev/null\n+++ b/t/t7002-grep.sh\n@@ -0,0 +1,45 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2006 Junio C Hamano\n+#\n+\n+test_description='git grep -w\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\t{\n+\t\techo foo mmap bar\n+\t\techo foo_mmap bar\n+\t\techo foo_mmap bar mmap\n+\t\techo foo mmap bar_mmap\n+\t\techo foo_mmap bar mmap baz\n+\t} >file &&\n+\tgit add file &&\n+\tgit commit -m initial\n+'\n+\n+test_expect_success 'grep -w HEAD' '\n+\tgit grep -n -w -e mmap HEAD >actual &&\n+\t{\n+\t\techo HEAD:file:1:foo mmap bar\n+\t\techo HEAD:file:3:foo_mmap bar mmap\n+\t\techo HEAD:file:4:foo mmap bar_mmap\n+\t\techo HEAD:file:5:foo_mmap bar mmap baz\n+\t} >expected &&\n+\tdiff expected actual\n+'\n+\n+test_expect_success 'grep -w in working tree' '\n+\tgit grep -n -w -e mmap >actual &&\n+\t{\n+\t\techo file:1:foo mmap bar\n+\t\techo file:3:foo_mmap bar mmap\n+\t\techo file:4:foo mmap bar_mmap\n+\t\techo file:5:foo_mmap bar mmap baz\n+\t} >expected &&\n+\tdiff expected actual\n+'\n+\n+test_done\n"},{"id":"24837","messageId":"118833cc0608051219q7e19800alc05870058973c2e@mail.gmail.com","threadId":"5140","inReplyTo":"7vwt9ng3t1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC/PATCH] Fix \"grep -w\"","fromName":"Morten Welinder","fromEmail":"mwelinder@gmail.com","sentAt":"2006-08-05T19:19:02Z","receivedAt":"2006-08-05T19:19:02Z","isPatch":true,"sender":{"key":"mwelinder@gmail.com","avatar":null},"body":"1. Are you sure that going to the end of the first match is correct?\nIt seems to\n   me that this will skip matches.  Say you search for \".*\" on a line that reads\n   \" xxx\".\n\n2. What about \"^\"?\n\n3. What about empty matches?  That could take a while...\n\nMorten\n"},{"id":"24848","messageId":"7vlkq2dh6b.fsf@assigned-by-dhcp.cox.net","threadId":"5140","inReplyTo":"118833cc0608051219q7e19800alc05870058973c2e@mail.gmail.com","subject":"Re: [RFC/PATCH] Fix \"grep -w\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-05T21:08:28Z","receivedAt":"2006-08-05T21:08:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Morten Welinder\" <mwelinder@gmail.com> writes:\n\n> 1. Are you sure that going to the end of the first match is correct?\n> It seems to me that this will skip matches.  Say you search\n> for \".*\" on a line that reads\n>   \" xxx\".\n\nIt is fine for your example, I think.  .* matches the entire\nline the first time, and BOL and EOL are defined to be word\nboundaries.  But you are right.  If the pattern is \"x xx* x\" and\nthe line is \"x x xx x\", the first round would match the first 5\nbytes, we find that 6th byte 'x' makes it not a word boundary,\nand redoing the match starting at 6th is a wrong thing to do.\nWe should find \"x xx x\" starting at the 3rd byte.\n\n> 2. What about \"^\"?\n\nThe pattern would not match the second time anyway, so I do not\nthink it is such a big deal.\n\nBut there is another bug I just spotted.  git grep -w -e '^x'\nmatches line \"xxx\" (when not cheating with external grep).\n\n> 3. What about empty matches?  That could take a while...\n\nTrue.  So we would need to make sure we advance at least one.\n"},{"id":"24880","messageId":"7vy7u2b6ly.fsf@assigned-by-dhcp.cox.net","threadId":"5140","inReplyTo":"7vlkq2dh6b.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC/PATCH] Fix \"grep -w\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-06T08:39:37Z","receivedAt":"2006-08-06T08:39:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We used to find the first match of the pattern and then if the\nmatch is not for the entire word, declared that the whole line\ndoes not match.\n\nBut that is wrong.  The command \"git grep -w -e mmap\" should\nfind that a line \"foo_mmap bar mmap baz\" matches, by tring the\nsecond instance of pattern \"mmap\" on the same line.\n\nProblems an earlier round of \"fix\" had were pointed out by Morten\nWelinder, which have been incorporated in the t7002 tests.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n * Second try.\n\n builtin-grep.c  |   35 ++++++++++++++++-------\n t/t7002-grep.sh |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 110 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 69b7c48..93b7e07 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -410,8 +410,10 @@ static int fixmatch(const char *pattern,\n static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol, char *eol)\n {\n \tint hit = 0;\n+\tint at_true_bol = 1;\n \tregmatch_t pmatch[10];\n \n+ again:\n \tif (!opt->fixed) {\n \t\tregex_t *exp = &p->regexp;\n \t\thit = !regexec(exp, bol, ARRAY_SIZE(pmatch),\n@@ -422,22 +424,35 @@ static int match_one_pattern(struct grep\n \t}\n \n \tif (hit && opt->word_regexp) {\n-\t\t/* Match beginning must be either\n-\t\t * beginning of the line, or at word\n-\t\t * boundary (i.e. the last char must\n-\t\t * not be alnum or underscore).\n-\t\t */\n \t\tif ((pmatch[0].rm_so < 0) ||\n \t\t    (eol - bol) <= pmatch[0].rm_so ||\n \t\t    (pmatch[0].rm_eo < 0) ||\n \t\t    (eol - bol) < pmatch[0].rm_eo)\n \t\t\tdie(\"regexp returned nonsense\");\n-\t\tif (pmatch[0].rm_so != 0 &&\n-\t\t    word_char(bol[pmatch[0].rm_so-1]))\n-\t\t\thit = 0;\n-\t\tif (pmatch[0].rm_eo != (eol-bol) &&\n-\t\t    word_char(bol[pmatch[0].rm_eo]))\n+\n+\t\t/* Match beginning must be either beginning of the\n+\t\t * line, or at word boundary (i.e. the last char must\n+\t\t * not be a word char).  Similarly, match end must be\n+\t\t * either end of the line, or at word boundary\n+\t\t * (i.e. the next char must not be a word char).\n+\t\t */\n+\t\tif ( ((pmatch[0].rm_so == 0 && at_true_bol) ||\n+\t\t      !word_char(bol[pmatch[0].rm_so-1])) &&\n+\t\t     ((pmatch[0].rm_eo == (eol-bol)) ||\n+\t\t      !word_char(bol[pmatch[0].rm_eo])) )\n+\t\t\t;\n+\t\telse\n \t\t\thit = 0;\n+\n+\t\tif (!hit && pmatch[0].rm_so + bol + 1 < eol) {\n+\t\t\t/* There could be more than one match on the\n+\t\t\t * line, and the first match might not be\n+\t\t\t * strict word match.  But later ones could be!\n+\t\t\t */\n+\t\t\tbol = pmatch[0].rm_so + bol + 1;\n+\t\t\tat_true_bol = 0;\n+\t\t\tgoto again;\n+\t\t}\n \t}\n \treturn hit;\n }\ndiff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\nnew file mode 100755\nindex 0000000..00a7d76\n--- /dev/null\n+++ b/t/t7002-grep.sh\n@@ -0,0 +1,85 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2006 Junio C Hamano\n+#\n+\n+test_description='git grep -w\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\t{\n+\t\techo foo mmap bar\n+\t\techo foo_mmap bar\n+\t\techo foo_mmap bar mmap\n+\t\techo foo mmap bar_mmap\n+\t\techo foo_mmap bar mmap baz\n+\t} >file &&\n+\techo x x xx x >x &&\n+\techo y yy >y &&\n+\techo zzz > z &&\n+\tgit add file x y z &&\n+\tgit commit -m initial\n+'\n+\n+for H in HEAD ''\n+do\n+\tcase \"$H\" in\n+\tHEAD)\tHC='HEAD:' L='HEAD' ;;\n+\t'')\tHC= L='in working tree' ;;\n+\tesac\n+\n+\ttest_expect_success \"grep -w $L\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:foo mmap bar\n+\t\t\techo ${HC}file:3:foo_mmap bar mmap\n+\t\t\techo ${HC}file:4:foo mmap bar_mmap\n+\t\t\techo ${HC}file:5:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep -n -w -e mmap $H >actual &&\n+\t\tdiff expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (x)\" '\n+\t\t{\n+\t\t\techo ${HC}x:1:x x xx x\n+\t\t} >expected &&\n+\t\tgit grep -n -w -e \"x xx* x\" $H >actual &&\n+\t\tdiff expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (y-1)\" '\n+\t\t{\n+\t\t\techo ${HC}y:1:y yy\n+\t\t} >expected &&\n+\t\tgit grep -n -w -e \"^y\" $H >actual &&\n+\t\tdiff expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (y-2)\" '\n+\t\t: >expected &&\n+\t\tif git grep -n -w -e \"^y y\" $H >actual\n+\t\tthen\n+\t\t\techo should not have matched\n+\t\t\tcat actual\n+\t\t\tfalse\n+\t\telse\n+\t\t\tdiff expected actual\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (z)\" '\n+\t\t: >expected &&\n+\t\tif git grep -n -w -e \"^z\" $H >actual\n+\t\tthen\n+\t\t\techo should not have matched\n+\t\t\tcat actual\n+\t\t\tfalse\n+\t\telse\n+\t\t\tdiff expected actual\n+\t\tfi\n+\t'\n+done\n+\n+test_done\n-- \n1.4.2.rc3.g19a8a\n"}]}