{"thread":{"id":"30553","subject":"git grep -F doesn't behave like grep -F?","startedAt":"2012-05-18T11:00:17Z","lastAt":"2012-05-21T16:10:09Z","messageCount":8,"participants":["Torne (Richard Coles)","René Scharfe","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"191654","messageId":"CAEV-rjc0PtuQZei95_24=Ou=mZZxA0Lsr6boXGrGy3z40otkNQ@mail.gmail.com","threadId":"30553","inReplyTo":null,"subject":"git grep -F doesn't behave like grep -F?","fromName":"Torne (Richard Coles)","fromEmail":"torne@google.com","sentAt":"2012-05-18T11:00:17Z","receivedAt":"2012-05-18T11:00:17Z","isPatch":false,"sender":{"key":"torne@google.com","avatar":null},"body":"Hi folks,\n\ngit grep -F is documented as: \"Use fixed strings for patterns (don’t\ninterpret pattern as a regex).\"\n\nwhereas grep -F is documented as \"Interpret PATTERN as a list of\nfixed strings, separated by newlines, any of which is to be\nmatched.\"\n\nThis accurately describes how they behave, which means that git grep\n-F with a pattern containing newlines never matches anything (at least\nas far as I can see). Is this intentional, or an oversight? The\nability to grep -F for a list (e.g. the output of another grep) is\npretty handy...\n\n-- \nTorne (Richard Coles)\ntorne@google.com\n"},{"id":"191659","messageId":"4FB6426C.7040202@lsrfire.ath.cx","threadId":"30553","inReplyTo":"CAEV-rjc0PtuQZei95_24=Ou=mZZxA0Lsr6boXGrGy3z40otkNQ@mail.gmail.com","subject":"Re: git grep -F doesn't behave like grep -F?","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-05-18T12:37:00Z","receivedAt":"2012-05-18T12:37:00Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 18.05.2012 13:00, schrieb Torne (Richard Coles):\n> Hi folks,\n>\n> git grep -F is documented as: \"Use fixed strings for patterns (don’t\n> interpret pattern as a regex).\"\n>\n> whereas grep -F is documented as \"Interpret PATTERN as a list of\n> fixed strings, separated by newlines, any of which is to be\n> matched.\"\n>\n> This accurately describes how they behave, which means that git grep\n> -F with a pattern containing newlines never matches anything (at least\n> as far as I can see). Is this intentional, or an oversight? The\n> ability to grep -F for a list (e.g. the output of another grep) is\n> pretty handy...\n\nYou could use -f- (read patterns from stdin).\n\nThat said, it looks like a missing feature to me -- at least I didn't \nknow that grep -F takes newline separated lists of search strings. And \nthis doesn't seem to be restricted to invocations with -F, only; a plain \ngrep with regexps does it as well.\n\nRené\n"},{"id":"191660","messageId":"CAEV-rjd=WjLu8e+UCnAHVxg7DTLWe+YrEO_Gs2rh5Oy1=KA5sw@mail.gmail.com","threadId":"30553","inReplyTo":"4FB6426C.7040202@lsrfire.ath.cx","subject":"Re: git grep -F doesn't behave like grep -F?","fromName":"Torne (Richard Coles)","fromEmail":"torne@google.com","sentAt":"2012-05-18T12:41:44Z","receivedAt":"2012-05-18T12:41:44Z","isPatch":false,"sender":{"key":"torne@google.com","avatar":null},"body":"On 18 May 2012 13:37, René Scharfe wrote:\n> Am 18.05.2012 13:00, schrieb Torne (Richard Coles):\n>\n>> Hi folks,\n>>\n>> git grep -F is documented as: \"Use fixed strings for patterns (don’t\n>> interpret pattern as a regex).\"\n>>\n>> whereas grep -F is documented as \"Interpret PATTERN as a  list  of\n>> fixed  strings,  separated  by newlines,  any  of  which is to be\n>> matched.\"\n>>\n>> This accurately describes how they behave, which means that git grep\n>> -F with a pattern containing newlines never matches anything (at least\n>> as far as I can see). Is this intentional, or an oversight? The\n>> ability to grep -F for a list (e.g. the output of another grep) is\n>> pretty handy...\n>\n>\n> You could use -f- (read patterns from stdin).\n\nAh, yes, rewriting\n\ngit grep -F \"`git grep -o someregex`\"\n\nas\n\ngit grep -o someregex | git grep -F -f-\n\nworks, but it's not how I immediately think to do it :)\n\n> That said, it looks like a missing feature to me -- at least I didn't know\n> that grep -F takes newline separated lists of search strings.  And this\n> doesn't seem to be restricted to invocations with -F, only; a plain grep\n> with regexps does it as well.\n\nYeah, it doesn't seem like adding it would break anything; patterns\nwith newlines don't match any lines by definition currently :)\n\n-- \nTorne (Richard Coles)\ntorne@google.com\n"},{"id":"191762","messageId":"4FB90087.2020606@lsrfire.ath.cx","threadId":"30553","inReplyTo":"CAEV-rjd=WjLu8e+UCnAHVxg7DTLWe+YrEO_Gs2rh5Oy1=KA5sw@mail.gmail.com","subject":"[PATCH 1/3] grep: factor out create_grep_pat()","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-05-20T14:32:39Z","receivedAt":"2012-05-20T14:32:39Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add create_grep_pat(), a shared helper for all grep pattern allocation\nand initialization needs.\n\nSigned-off-by: Rene Scharfe \n---\n grep.c | 28 +++++++++++++++++-----------\n 1 file changed, 17 insertions(+), 11 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex f8ffa46..a673ced 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -3,15 +3,26 @@\n #include \"userdiff.h\"\n #include \"xdiff-interface.h\"\n \n-void append_header_grep_pattern(struct grep_opt *opt, enum grep_header_field field, const char *pat)\n+static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,\n+\t\t\t\t\tconst char *origin, int no,\n+\t\t\t\t\tenum grep_pat_token t,\n+\t\t\t\t\tenum grep_header_field field)\n {\n \tstruct grep_pat *p = xcalloc(1, sizeof(*p));\n \tp->pattern = pat;\n-\tp->patternlen = strlen(pat);\n-\tp->origin = \"header\";\n-\tp->no = 0;\n-\tp->token = GREP_PATTERN_HEAD;\n+\tp->patternlen = patlen;\n+\tp->origin = origin;\n+\tp->no = no;\n+\tp->token = t;\n \tp->field = field;\n+\treturn p;\n+}\n+\n+void append_header_grep_pattern(struct grep_opt *opt,\n+\t\t\t\tenum grep_header_field field, const char *pat)\n+{\n+\tstruct grep_pat *p = create_grep_pat(pat, strlen(pat), \"header\", 0,\n+\t\t\t\t\t GREP_PATTERN_HEAD, field);\n \t*opt->header_tail = p;\n \topt->header_tail = &p->next;\n \tp->next = NULL;\n@@ -26,12 +37,7 @@ void append_grep_pattern(struct grep_opt *opt, const char *pat,\n void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen,\n \t\t const char *origin, int no, enum grep_pat_token t)\n {\n-\tstruct grep_pat *p = xcalloc(1, sizeof(*p));\n-\tp->pattern = pat;\n-\tp->patternlen = patlen;\n-\tp->origin = origin;\n-\tp->no = no;\n-\tp->token = t;\n+\tstruct grep_pat *p = create_grep_pat(pat, patlen, origin, no, t, 0);\n \t*opt->pattern_tail = p;\n \topt->pattern_tail = &p->next;\n \tp->next = NULL;\n-- \n1.7.10.2\n"},{"id":"191764","messageId":"4FB90096.9070600@lsrfire.ath.cx","threadId":"30553","inReplyTo":"CAEV-rjd=WjLu8e+UCnAHVxg7DTLWe+YrEO_Gs2rh5Oy1=KA5sw@mail.gmail.com","subject":"[PATCH 2/3] grep: factor out do_append_grep_pat()","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-05-20T14:32:54Z","receivedAt":"2012-05-20T14:32:54Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add do_append_grep_pat() as a shared function for adding patterns to\nthe header pattern list and the general pattern list.\n\nSigned-off-by: Rene Scharfe \n---\nOur first three star function. :)\n\n grep.c | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex a673ced..f961c2e 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -18,14 +18,19 @@ static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,\n \treturn p;\n }\n \n+static void do_append_grep_pat(struct grep_pat ***tail, struct grep_pat *p)\n+{\n+\t**tail = p;\n+\t*tail = &p->next;\n+\tp->next = NULL;\n+}\n+\n void append_header_grep_pattern(struct grep_opt *opt,\n \t\t\t\tenum grep_header_field field, const char *pat)\n {\n \tstruct grep_pat *p = create_grep_pat(pat, strlen(pat), \"header\", 0,\n \t\t\t\t\t GREP_PATTERN_HEAD, field);\n-\t*opt->header_tail = p;\n-\topt->header_tail = &p->next;\n-\tp->next = NULL;\n+\tdo_append_grep_pat(&opt->header_tail, p);\n }\n \n void append_grep_pattern(struct grep_opt *opt, const char *pat,\n@@ -38,9 +43,7 @@ void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen,\n \t\t const char *origin, int no, enum grep_pat_token t)\n {\n \tstruct grep_pat *p = create_grep_pat(pat, patlen, origin, no, t, 0);\n-\t*opt->pattern_tail = p;\n-\topt->pattern_tail = &p->next;\n-\tp->next = NULL;\n+\tdo_append_grep_pat(&opt->pattern_tail, p);\n }\n \n struct grep_opt *grep_opt_dup(const struct grep_opt *opt)\n-- \n1.7.10.2\n"},{"id":"191763","messageId":"4FB900A3.9050309@lsrfire.ath.cx","threadId":"30553","inReplyTo":"CAEV-rjd=WjLu8e+UCnAHVxg7DTLWe+YrEO_Gs2rh5Oy1=KA5sw@mail.gmail.com","subject":"[PATCH 3/3] grep: support newline separated pattern list","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-05-20T14:33:07Z","receivedAt":"2012-05-20T14:33:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Currently, patterns that contain newline characters don't match anything\nwhen given to git grep. Regular grep(1) interprets patterns as lists of\nnewline separated search strings instead.\n\nImplement this functionality by creating and inserting extra grep_pat\nstructures for patterns consisting of multiple lines when appending to\nthe pattern lists. For simplicity, all pattern strings are duplicated.\nThe original pattern is truncated in place to make it contain only the\nfirst line.\n\nRequested-by: Torne (Richard Coles) \nSigned-off-by: Rene Scharfe \n---\n Documentation/git-grep.txt | 4 +++-\n grep.c | 33 ++++++++++++++++++++++++++++++++-\n grep.h | 2 +-\n t/t7810-grep.sh | 5 +++++\n 4 files changed, 41 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 4785f1c..3bec036 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -31,7 +31,9 @@ SYNOPSIS\n DESCRIPTION\n -----------\n Look for specified patterns in the tracked files in the work tree, blobs\n-registered in the index file, or blobs in given tree objects.\n+registered in the index file, or blobs in given tree objects. Patterns\n+are lists of one or more search expressions separated by newline\n+characters. An empty string as search expression matches all lines.\n \n \n CONFIGURATION\ndiff --git a/grep.c b/grep.c\nindex f961c2e..04e3ec6 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -9,7 +9,7 @@ static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,\n \t\t\t\t\tenum grep_header_field field)\n {\n \tstruct grep_pat *p = xcalloc(1, sizeof(*p));\n-\tp->pattern = pat;\n+\tp->pattern = xmemdupz(pat, patlen);\n \tp->patternlen = patlen;\n \tp->origin = origin;\n \tp->no = no;\n@@ -23,6 +23,36 @@ static void do_append_grep_pat(struct grep_pat ***tail, struct grep_pat *p)\n \t**tail = p;\n \t*tail = &p->next;\n \tp->next = NULL;\n+\n+\tswitch (p->token) {\n+\tcase GREP_PATTERN: /* atom */\n+\tcase GREP_PATTERN_HEAD:\n+\tcase GREP_PATTERN_BODY:\n+\t\tfor (;;) {\n+\t\t\tstruct grep_pat *new_pat;\n+\t\t\tsize_t len = 0;\n+\t\t\tchar *cp = p->pattern + p->patternlen, *nl = NULL;\n+\t\t\twhile (++len <= p->patternlen) {\n+\t\t\t\tif (*(--cp) == '\\n') {\n+\t\t\t\t\tnl = cp;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tif (!nl)\n+\t\t\t\tbreak;\n+\t\t\tnew_pat = create_grep_pat(nl + 1, len - 1, p->origin,\n+\t\t\t\t\t\t p->no, p->token, p->field);\n+\t\t\tnew_pat->next = p->next;\n+\t\t\tif (!p->next)\n+\t\t\t\t*tail = &new_pat->next;\n+\t\t\tp->next = new_pat;\n+\t\t\t*nl = '\\0';\n+\t\t\tp->patternlen -= len;\n+\t\t}\n+\t\tbreak;\n+\tdefault:\n+\t\tbreak;\n+\t}\n }\n \n void append_header_grep_pattern(struct grep_opt *opt,\n@@ -439,6 +469,7 @@ void free_grep_patterns(struct grep_opt *opt)\n \t\t\t\tfree_pcre_regexp(p);\n \t\t\telse\n \t\t\t\tregfree(&p->regexp);\n+\t\t\tfree(p->pattern);\n \t\t\tbreak;\n \t\tdefault:\n \t\t\tbreak;\ndiff --git a/grep.h b/grep.h\nindex 36e49d8..ed7de6b 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -38,7 +38,7 @@ struct grep_pat {\n \tconst char *origin;\n \tint no;\n \tenum grep_pat_token token;\n-\tconst char *pattern;\n+\tchar *pattern;\n \tsize_t patternlen;\n \tenum grep_header_field field;\n \tregex_t regexp;\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex d9ad633..24e9b19 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -351,6 +351,11 @@ test_expect_success 'grep -f, multiple patterns' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'grep, multiple patterns' '\n+\tgit grep \"$(cat patterns)\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n cat >expected < writes:\n\n> Currently, patterns that contain newline characters don't match anything\n> when given to git grep. Regular grep(1) interprets patterns as lists of\n> newline separated search strings instead.\n>\n> Implement this functionality by creating and inserting extra grep_pat\n> structures for patterns consisting of multiple lines when appending to\n> the pattern lists. For simplicity, all pattern strings are duplicated.\n> The original pattern is truncated in place to make it contain only the\n> first line.\n\nThanks for a fix; I am assuming that the duplication for simplicity is not\nfor simplified allocation but primarily for a simpler freeing?\n"},{"id":"191816","messageId":"4FBA68E1.60908@lsrfire.ath.cx","threadId":"30553","inReplyTo":"7vobpia3l3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] grep: support newline separated pattern list","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2012-05-21T16:10:09Z","receivedAt":"2012-05-21T16:10:09Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 21.05.2012 00:29, schrieb Junio C Hamano:\n> René Scharfe writes:\n> \n>> Currently, patterns that contain newline characters don't match anything\n>> when given to git grep. Regular grep(1) interprets patterns as lists of\n>> newline separated search strings instead.\n>>\n>> Implement this functionality by creating and inserting extra grep_pat\n>> structures for patterns consisting of multiple lines when appending to\n>> the pattern lists. For simplicity, all pattern strings are duplicated.\n>> The original pattern is truncated in place to make it contain only the\n>> first line.\n> \n> Thanks for a fix; I am assuming that the duplication for simplicity is not\n> for simplified allocation but primarily for a simpler freeing?\n\nWhen we split a search string into multiple parts, we could allocate only\nthe new parts and remember which strings were allocated, so that we could\nlater free them (and only them). Or we could allocate and leak them, as\nthere aren't that many and we keep them during the whole run of the program\nanyway. Or we could do without any allocations if all match backends\nrespected length limited strings like kwset does. Or only allocate\nnon-fixed pattern.\n\nAllocating any and all pattern strings and freeing them at the end, thus\ndisregarding these considerations, is simpler, cleaner and still doesn't\ncause excessive memory usage.\n\nThat reminds me that we can now have this bonus patch:\n\n-- >8 --\nSubject: [PATCH 4/3] grep: stop leaking line strings with -f\n\nWhen reading patterns from a file, we pass the lines as allocated string\nbuffers to append_grep_pat() and never free them. That's not a problem\nbecause they are needed until the program ends anyway.\n\nHowever, now that the function duplicates the pattern string, we can\nreuse the strbuf after calling that function. This simplifies the code\na bit and plugs a minor memory leak.\n\nSigned-off-by: Rene Scharfe \n---\n builtin/grep.c | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 643938d..fe1726f 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -600,15 +600,12 @@ static int file_callback(const struct option *opt, const char *arg, int unset)\n \tif (!patterns)\n \t\tdie_errno(_(\"cannot open '%s'\"), arg);\n \twhile (strbuf_getline(&sb, patterns, '\\n') == 0) {\n-\t\tchar *s;\n-\t\tsize_t len;\n-\n \t\t/* ignore empty line like grep does */\n \t\tif (sb.len == 0)\n \t\t\tcontinue;\n \n-\t\ts = strbuf_detach(&sb, &len);\n-\t\tappend_grep_pat(grep_opt, s, len, arg, ++lno, GREP_PATTERN);\n+\t\tappend_grep_pat(grep_opt, sb.buf, sb.len, arg, ++lno,\n+\t\t\t\tGREP_PATTERN);\n \t}\n \tif (!from_stdin)\n \t\tfclose(patterns);\n-- \n1.7.10.2\n"}]}