{"thread":{"id":"21432","subject":"[PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","startedAt":"2009-10-30T01:21:59Z","lastAt":"2009-11-17T07:38:45Z","messageCount":13,"participants":["Brian Collins","Jeff King","Junio C Hamano","Nanako Shiraishi","Brandon Casey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"126348","messageId":"B7C4E16C-B15D-4A7B-873A-B6BD0FDAD8C8@gmail.com","threadId":"21432","inReplyTo":null,"subject":"[PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Brian Collins","fromEmail":"bricollins@gmail.com","sentAt":"2009-10-30T01:21:59Z","receivedAt":"2009-10-30T01:21:59Z","isPatch":true,"sender":{"key":"bricollins@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22232?v=4"},"body":"You will have to excuse me, this is my first patch and I don't know if \nthis is the right place to post this. Apologies in advance if I'm in \nthe wrong place.\n\ngit-grep currently throws an error when you combine the -F and -i \nflags. This isn't in line with how GNU grep handles it. This patch \nallows the simultaneous use of those flags.\n\n---\n builtin-grep.c | 8 +++++---\n grep.c | 16 ++++++++++++----\n grep.h | 2 ++\n 3 files changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 761799d..c73f05b 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -367,7 +367,7 @@ static int external_grep(struct grep_opt *opt, \nconst char **paths, int cached)\n \t\tpush_arg(\"-h\");\n \tif (opt->regflags & REG_EXTENDED)\n \t\tpush_arg(\"-E\");\n-\tif (opt->regflags & REG_ICASE)\n+\tif (opt->caseless)\n \t\tpush_arg(\"-i\");\n \tif (opt->binary == GREP_BINARY_NOMATCH)\n \t\tpush_arg(\"-I\");\n@@ -706,8 +706,8 @@ int cmd_grep(int argc, const char **argv, const \nchar *prefix)\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOLEAN('v', \"invert-match\", &opt.invert,\n \t\t\t\"show non-matching lines\"),\n-\t\tOPT_BIT('i', \"ignore-case\", &opt.regflags,\n-\t\t\t\"case insensitive matching\", REG_ICASE),\n+\t\tOPT_BOOLEAN('i', \"ignore-case\", &opt.caseless,\n+\t\t\t\"case insensitive matching\"),\n \t\tOPT_BOOLEAN('w', \"word-regexp\", &opt.word_regexp,\n \t\t\t\"match patterns only at word boundaries\"),\n \t\tOPT_SET_INT('a', \"text\", &opt.binary,\n@@ -830,6 +830,8 @@ int cmd_grep(int argc, const char **argv, const \nchar *prefix)\n \t\texternal_grep_allowed = 0;\n \tif (!opt.pattern_list)\n \t\tdie(\"no pattern given.\");\n+\tif (!opt.fixed && opt.caseless)\n+\t\topt.regflags |= REG_ICASE;\n \tif ((opt.regflags != REG_NEWLINE) && opt.fixed)\n \t\tdie(\"cannot mix --fixed-strings and regexp\");\n \tcompile_grep_patterns(&opt);\ndiff --git a/grep.c b/grep.c\nindex 5d162da..d8f14be 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -41,7 +41,8 @@ static void compile_regexp(struct grep_pat *p, \nstruct grep_opt *opt)\n \tint err;\n\n \tp->word_regexp = opt->word_regexp;\n-\n+\tp->caseless = opt->caseless;\n+\t\n \tif (opt->fixed || is_fixed(p->pattern))\n \t\tp->fixed = 1;\n \tif (opt->regflags & REG_ICASE)\n@@ -262,9 +263,16 @@ static void show_name(struct grep_opt *opt, const \nchar *name)\n \tprintf(\"%s%c\", name, opt->null_following_name ? '\\0' : '\\n');\n }\n\n-static int fixmatch(const char *pattern, char *line, regmatch_t *match)\n+\n+static int fixmatch(const char *pattern, char *line, int caseless, \nregmatch_t *match)\n {\n-\tchar *hit = strstr(line, pattern);\n+\tchar *hit;\n+\tif (caseless) {\n+\t\thit = strcasestr(line, pattern);\n+\t} else {\n+\t\thit = strstr(line, pattern);\n+\t}\n+\t\n \tif (!hit) {\n \t\tmatch->rm_so = match->rm_eo = -1;\n \t\treturn REG_NOMATCH;\n@@ -326,7 +334,7 @@ static int match_one_pattern(struct grep_pat *p, \nchar *bol, char *eol,\n\n again:\n \tif (p->fixed)\n-\t\thit = !fixmatch(p->pattern, bol, pmatch);\n+\t\thit = !fixmatch(p->pattern, bol, p->caseless, pmatch);\n \telse\n \t\thit = !regexec(&p->regexp, bol, 1, pmatch, eflags);\n\ndiff --git a/grep.h b/grep.h\nindex f6eecc6..24b7d44 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -32,6 +32,7 @@ struct grep_pat {\n \tenum grep_header_field field;\n \tregex_t regexp;\n \tunsigned fixed:1;\n+\tunsigned caseless:1;\n \tunsigned word_regexp:1;\n };\n\n@@ -64,6 +65,7 @@ struct grep_opt {\n \tregex_t regexp;\n \tint linenum;\n \tint invert;\n+\tint caseless;\n \tint status_only;\n \tint name_only;\n \tint unmatch_name_only;\n-- \n1.6.4.4\n"},{"id":"126963","messageId":"20091106084855.GA20964@coredump.intra.peff.net","threadId":"21432","inReplyTo":"B7C4E16C-B15D-4A7B-873A-B6BD0FDAD8C8@gmail.com","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-06T08:48:55Z","receivedAt":"2009-11-06T08:48:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 29, 2009 at 06:21:59PM -0700, Brian Collins wrote:\n\n> You will have to excuse me, this is my first patch and I don't know\n> if this is the right place to post this. Apologies in advance if I'm\n> in the wrong place.\n\nYou're in the right place (though judging from the response, nobody\nseemed to find your patch all that interesting...).\n\n> git-grep currently throws an error when you combine the -F and -i\n> flags. This isn't in line with how GNU grep handles it. This patch\n> allows the simultaneous use of those flags.\n\nI don't see a reason not to allow this combination if our grep\nimplementation supports it. My only reservation would be that we\nsometimes call out to an external grep, and non-GNU grep might barf on\nthis. But I think that is OK, as the user should get a sane error from\nthe external grep.\n\n> builtin-grep.c | 8 +++++---\n> grep.c | 16 ++++++++++++----\n> grep.h | 2 ++\n\nTests? They help prove to us that your feature works, and also prevent\nus from accidentally breaking your feature in the future.\n\n-Peff\n"},{"id":"126968","messageId":"d1c0fbfa0911060122u3ffc85e7pf60a689cdfd85850@mail.gmail.com","threadId":"21432","inReplyTo":"20091106084855.GA20964@coredump.intra.peff.net","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Brian Collins","fromEmail":"bricollins@gmail.com","sentAt":"2009-11-06T09:22:35Z","receivedAt":"2009-11-06T09:22:35Z","isPatch":true,"sender":{"key":"bricollins@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22232?v=4"},"body":"2009/11/6 Jeff King :\n> You're in the right place (though judging from the response, nobody\n> seemed to find your patch all that interesting...).\n\nHeh, yeah it is a bit of a boring edge case but a TextMate plugin I am writing\nrequires this functionality.\n\n> Tests? They help prove to us that your feature works, and also prevent\n> us from accidentally breaking your feature in the future.\n\nAh yes that is what I was forgetting. Please see the amended patch including\ntest.\nThanks for your help\n\n\n---\n builtin-grep.c | 8 +++++---\n grep.c | 16 ++++++++++++----\n grep.h | 2 ++\n t/t7002-grep.sh | 9 +++++++++\n 4 files changed, 28 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 761799d..c73f05b 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -367,7 +367,7 @@ static int external_grep(struct grep_opt *opt,\nconst char **paths, int cached)\n \t\tpush_arg(\"-h\");\n \tif (opt->regflags & REG_EXTENDED)\n \t\tpush_arg(\"-E\");\n-\tif (opt->regflags & REG_ICASE)\n+\tif (opt->caseless)\n \t\tpush_arg(\"-i\");\n \tif (opt->binary == GREP_BINARY_NOMATCH)\n \t\tpush_arg(\"-I\");\n@@ -706,8 +706,8 @@ int cmd_grep(int argc, const char **argv, const\nchar *prefix)\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOLEAN('v', \"invert-match\", &opt.invert,\n \t\t\t\"show non-matching lines\"),\n-\t\tOPT_BIT('i', \"ignore-case\", &opt.regflags,\n-\t\t\t\"case insensitive matching\", REG_ICASE),\n+\t\tOPT_BOOLEAN('i', \"ignore-case\", &opt.caseless,\n+\t\t\t\"case insensitive matching\"),\n \t\tOPT_BOOLEAN('w', \"word-regexp\", &opt.word_regexp,\n \t\t\t\"match patterns only at word boundaries\"),\n \t\tOPT_SET_INT('a', \"text\", &opt.binary,\n@@ -830,6 +830,8 @@ int cmd_grep(int argc, const char **argv, const\nchar *prefix)\n \t\texternal_grep_allowed = 0;\n \tif (!opt.pattern_list)\n \t\tdie(\"no pattern given.\");\n+\tif (!opt.fixed && opt.caseless)\n+\t\topt.regflags |= REG_ICASE;\n \tif ((opt.regflags != REG_NEWLINE) && opt.fixed)\n \t\tdie(\"cannot mix --fixed-strings and regexp\");\n \tcompile_grep_patterns(&opt);\ndiff --git a/grep.c b/grep.c\nindex 5d162da..d8f14be 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -41,7 +41,8 @@ static void compile_regexp(struct grep_pat *p,\nstruct grep_opt *opt)\n \tint err;\n\n \tp->word_regexp = opt->word_regexp;\n-\n+\tp->caseless = opt->caseless;\n+\t\n \tif (opt->fixed || is_fixed(p->pattern))\n \t\tp->fixed = 1;\n \tif (opt->regflags & REG_ICASE)\n@@ -262,9 +263,16 @@ static void show_name(struct grep_opt *opt, const\nchar *name)\n \tprintf(\"%s%c\", name, opt->null_following_name ? '\\0' : '\\n');\n }\n\n-static int fixmatch(const char *pattern, char *line, regmatch_t *match)\n+\n+static int fixmatch(const char *pattern, char *line, int caseless,\nregmatch_t *match)\n {\n-\tchar *hit = strstr(line, pattern);\n+\tchar *hit;\n+\tif (caseless) {\n+\t\thit = strcasestr(line, pattern);\n+\t} else {\n+\t\thit = strstr(line, pattern);\n+\t}\n+\t\n \tif (!hit) {\n \t\tmatch->rm_so = match->rm_eo = -1;\n \t\treturn REG_NOMATCH;\n@@ -326,7 +334,7 @@ static int match_one_pattern(struct grep_pat *p,\nchar *bol, char *eol,\n\n again:\n \tif (p->fixed)\n-\t\thit = !fixmatch(p->pattern, bol, pmatch);\n+\t\thit = !fixmatch(p->pattern, bol, p->caseless, pmatch);\n \telse\n \t\thit = !regexec(&p->regexp, bol, 1, pmatch, eflags);\n\ndiff --git a/grep.h b/grep.h\nindex f6eecc6..24b7d44 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -32,6 +32,7 @@ struct grep_pat {\n \tenum grep_header_field field;\n \tregex_t regexp;\n \tunsigned fixed:1;\n+\tunsigned caseless:1;\n \tunsigned word_regexp:1;\n };\n\n@@ -64,6 +65,7 @@ struct grep_opt {\n \tregex_t regexp;\n \tint linenum;\n \tint invert;\n+\tint caseless;\n \tint status_only;\n \tint name_only;\n \tint unmatch_name_only;\ndiff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\nindex ae56a36..87b47dd 100755\n--- a/t/t7002-grep.sh\n+++ b/t/t7002-grep.sh\n@@ -345,4 +345,13 @@ test_expect_success 'grep from a subdirectory to\nsearch wider area (2)' '\n \t)\n '\n\n+cat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.6.4.4\n"},{"id":"126972","messageId":"7vbpjg0y8k.fsf@alter.siamese.dyndns.org","threadId":"21432","inReplyTo":"20091106084855.GA20964@coredump.intra.peff.net","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-06T10:00:11Z","receivedAt":"2009-11-06T10:00:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> git-grep currently throws an error when you combine the -F and -i\n>> flags. This isn't in line with how GNU grep handles it. This patch\n>> allows the simultaneous use of those flags.\n>\n> I don't see a reason not to allow this combination if our grep\n> implementation supports it. My only reservation would be that we\n> sometimes call out to an external grep, and non-GNU grep might barf on\n> this. But I think that is OK, as the user should get a sane error from\n> the external grep.\n\nI think that is OK but not for the reason you stated. The user should\nnever get such an error message.\n\nThe reason why I think this is OK is because the builder can choose to use\nNO_EXTERNAL_GREP if the external grep does not allow this combination.\n\nWe need to update comments on the Makefile if we are going to take this\npatch. Currently the description suggests three possible reasons you\nmight want to choose NO_EXTERNAL_GREP. \"lacks grep\" is obvious, \"slower\"\nis not about correctness, but \"is broken\" is way underspecified, and this\npatch adds one more reason to label your \"grep\" as broken.\n\nCurrently, SunOS and IRIX/IRIX64 are the only ones that specify\nNO_EXTERNAL_GREP, and I suspect both are defined due to \"is broken\", but\nwe do not tell the builder in what way they are considered broken, iow,\nwhat features we expect from the platform \"grep\", so somebody coming up\nwith a new port would be at loss.\n\nI suspect 01ae841 (SunOS grep does not understand -C nor -e, 2009-07-23)\nwould be a good starting point. Something like this...\n\n # Define NO_EXTERNAL_GREP if you don't want \"git grep\" to ever call\n # your external grep (e.g., if your system lacks grep, if its grep\n # does not support necessary features, or spawning external process is\n # slower than built-in grep git has). To be usable, your grep must\n # support -C (show n lines of context), -e (primarily\n # used to quote a pattern that begins with a dash), and use of -F and\n # -i at the same time. Otherwise define this.\n\nBut I didn't try hard to find out what _else_ we are depending on.\n\n> Tests? They help prove to us that your feature works, and also prevent\n> us from accidentally breaking your feature in the future.\n\nYeah, thanks. The latter is the primary purpose of test.\n"},{"id":"126974","messageId":"20091106101316.GA22549@coredump.intra.peff.net","threadId":"21432","inReplyTo":"7vbpjg0y8k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-06T10:13:17Z","receivedAt":"2009-11-06T10:13:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 06, 2009 at 02:00:11AM -0800, Junio C Hamano wrote:\n\n> > I don't see a reason not to allow this combination if our grep\n> > implementation supports it. My only reservation would be that we\n> > sometimes call out to an external grep, and non-GNU grep might barf on\n> > this. But I think that is OK, as the user should get a sane error from\n> > the external grep.\n> \n> I think that is OK but not for the reason you stated. The user should\n> never get such an error message.\n> \n> The reason why I think this is OK is because the builder can choose to use\n> NO_EXTERNAL_GREP if the external grep does not allow this combination.\n\nYes, I think that would be a sane thing to do (and I suspect anyone\nusing non-GNU grep is probably already doing it). But what I meant more\nwas that the _transition_ should be fine. If we start shipping with this\npatch but people haven't updated their build configuration, it is not\ngoing to break horribly; it should just produce an error, which is what\nit is doing now.\n\n> # Define NO_EXTERNAL_GREP if you don't want \"git grep\" to ever call\n> # your external grep (e.g., if your system lacks grep, if its grep\n> # does not support necessary features, or spawning external process is\n> # slower than built-in grep git has). To be usable, your grep must\n> # support -C (show n lines of context), -e (primarily\n> # used to quote a pattern that begins with a dash), and use of -F and\n> # -i at the same time. Otherwise define this.\n> \n> But I didn't try hard to find out what _else_ we are depending on.\n\nIt is not really _us_ depending on it. It is \"things the user wants to\ndo that _we_ support, but that their grep might not.\" So I don't think\nthere is much point in enumerating features. If their system grep\ndoesn't handle options that they want to use, then it won't work for\nthem. If they don't use them, then they will be fine.\n\nThough \"-e\" might be the exception, as I think we might use it\nunconditionally. But something like \"-F -i\" really depends on whether\nthe user wants to use it.\n\nSo I am fine with the text above, but I wouldn't worry too hard about\ntrying to come up with an exhaustive feature list.\n\n-Peff\n"},{"id":"127026","messageId":"7vzl6ztd8s.fsf@alter.siamese.dyndns.org","threadId":"21432","inReplyTo":"20091106101316.GA22549@coredump.intra.peff.net","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-07T00:00:51Z","receivedAt":"2009-11-07T00:00:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Nov 06, 2009 at 02:00:11AM -0800, Junio C Hamano wrote:\n>\n>> But I didn't try hard to find out what _else_ we are depending on.\n>\n> It is not really _us_ depending on it. It is \"things the user wants to\n> do that _we_ support, but that their grep might not.\" So I don't think\n> there is much point in enumerating features. If their system grep\n> doesn't handle options that they want to use, then it won't work for\n> them. If they don't use them, then they will be fine.\n>\n> Though \"-e\" might be the exception, as I think we might use it\n> unconditionally. But something like \"-F -i\" really depends on whether\n> the user wants to use it.\n\nYes and no.\n\nEven though we currently punt on a few platforms for simplicity and build\nwith NO_EXTERNAL_GREP, we could check if the set of options given are\nwithin the feature set of what the platform's grep understands and choose\nto spawn \"grep\" unless some options that are unsupported are used, in\nwhich case we fall back to the internal one.\n\nWe could certainly do something like this if it turns out to be a problem.\nAn invocation that does not use -F and -i together can still spawn\nexternal grep if that is faster.\n\nYou are correct about \"-e\". Our NO_EXTERNAL_GREP on SunOS cannot be\navoided.\n\n builtin-grep.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 1df25b0..2905f64 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -357,6 +357,9 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n \n \tif (opt->extended || (opt->relative && opt->prefix_length))\n \t\treturn -1;\n+\tif (NO_GREP_FIXED_IGNORE_CASE &&\n+\t opt->fixed && (opt->regflags & REG_ICASE))\n+\t\treturn -1;\n \tlen = nr = 0;\n \tpush_arg(\"grep\");\n \tif (opt->fixed)\n"},{"id":"127671","messageId":"20091116195050.6117@nanako3.lavabit.com","threadId":"21432","inReplyTo":"B7C4E16C-B15D-4A7B-873A-B6BD0FDAD8C8@gmail.com","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-11-16T10:50:50Z","receivedAt":"2009-11-16T10:50:50Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Brian Collins \n\n> You will have to excuse me, this is my first patch and I don't know if\n> this is the right place to post this. Apologies in advance if I'm in\n> the wrong place.\n>\n> git-grep currently throws an error when you combine the -F and -i\n> flags. This isn't in line with how GNU grep handles it. This patch\n> allows the simultaneous use of those flags.\n\nJunio, may I ask what happened to this patch?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"127687","messageId":"20091116162533.GA30737@coredump.intra.peff.net","threadId":"21432","inReplyTo":"20091116195050.6117@nanako3.lavabit.com","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-16T16:25:33Z","receivedAt":"2009-11-16T16:25:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 16, 2009 at 07:50:50PM +0900, Nanako Shiraishi wrote:\n\n> Quoting Brian Collins \n> \n> > You will have to excuse me, this is my first patch and I don't know if\n> > this is the right place to post this. Apologies in advance if I'm in\n> > the wrong place.\n> >\n> > git-grep currently throws an error when you combine the -F and -i\n> > flags. This isn't in line with how GNU grep handles it. This patch\n> > allows the simultaneous use of those flags.\n> \n> Junio, may I ask what happened to this patch?\n\nI think I owed it another review.\n\nI just looked it over and the idea and implementation look sane to me.\nThere were a few minor problems with the submission:\n\n 1. The patch was line-wrapped; I had to de-munge it manually to apply.\n\n 2. The original submission had cover-letter material mixed in with the\n commit message. The follow-up version had no commit message at all.\n\n 3. No signed-off-by. Brian, can you please acknowledge the DCO with a\n signoff?\n\n 4. The patch introduced some stray trailing whitespace.\n\n 5. There were a few style fixups, like omitting braces for a\n single-line conditional.\n\nTo save Junio time, here is a version that fixes all of those things. I\nthink it's probably worth applying to 'next'.\n\n-- >8 --\nFrom: Brian Collins \nDate: Fri, 6 Nov 2009 01:22:35 -0800\nSubject: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings\n\ngit-grep currently throws an error when you combine the -F\nand -i flags. This isn't in line with how GNU grep handles\nit. This patch allows the simultaneous use of those flags.\n\nSigned-off-by: Jeff King \n---\n builtin-grep.c | 8 +++++---\n grep.c | 13 ++++++++++---\n grep.h | 2 ++\n t/t7002-grep.sh | 9 +++++++++\n 4 files changed, 26 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 1df25b0..a2616fc 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -367,7 +367,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n \t\tpush_arg(\"-h\");\n \tif (opt->regflags & REG_EXTENDED)\n \t\tpush_arg(\"-E\");\n-\tif (opt->regflags & REG_ICASE)\n+\tif (opt->caseless)\n \t\tpush_arg(\"-i\");\n \tif (opt->binary == GREP_BINARY_NOMATCH)\n \t\tpush_arg(\"-I\");\n@@ -706,8 +706,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOLEAN('v', \"invert-match\", &opt.invert,\n \t\t\t\"show non-matching lines\"),\n-\t\tOPT_BIT('i', \"ignore-case\", &opt.regflags,\n-\t\t\t\"case insensitive matching\", REG_ICASE),\n+\t\tOPT_BOOLEAN('i', \"ignore-case\", &opt.caseless,\n+\t\t\t\"case insensitive matching\"),\n \t\tOPT_BOOLEAN('w', \"word-regexp\", &opt.word_regexp,\n \t\t\t\"match patterns only at word boundaries\"),\n \t\tOPT_SET_INT('a', \"text\", &opt.binary,\n@@ -830,6 +830,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\texternal_grep_allowed = 0;\n \tif (!opt.pattern_list)\n \t\tdie(\"no pattern given.\");\n+\tif (!opt.fixed && opt.caseless)\n+\t\topt.regflags |= REG_ICASE;\n \tif ((opt.regflags != REG_NEWLINE) && opt.fixed)\n \t\tdie(\"cannot mix --fixed-strings and regexp\");\n \tcompile_grep_patterns(&opt);\ndiff --git a/grep.c b/grep.c\nindex 5d162da..bbb0d18 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -41,6 +41,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \tint err;\n \n \tp->word_regexp = opt->word_regexp;\n+\tp->caseless = opt->caseless;\n \n \tif (opt->fixed || is_fixed(p->pattern))\n \t\tp->fixed = 1;\n@@ -262,9 +263,15 @@ static void show_name(struct grep_opt *opt, const char *name)\n \tprintf(\"%s%c\", name, opt->null_following_name ? '\\0' : '\\n');\n }\n \n-static int fixmatch(const char *pattern, char *line, regmatch_t *match)\n+\n+static int fixmatch(const char *pattern, char *line, int caseless, regmatch_t *match)\n {\n-\tchar *hit = strstr(line, pattern);\n+\tchar *hit;\n+\tif (caseless)\n+\t\thit = strcasestr(line, pattern);\n+\telse\n+\t\thit = strstr(line, pattern);\n+\n \tif (!hit) {\n \t\tmatch->rm_so = match->rm_eo = -1;\n \t\treturn REG_NOMATCH;\n@@ -326,7 +333,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \n again:\n \tif (p->fixed)\n-\t\thit = !fixmatch(p->pattern, bol, pmatch);\n+\t\thit = !fixmatch(p->pattern, bol, p->caseless, pmatch);\n \telse\n \t\thit = !regexec(&p->regexp, bol, 1, pmatch, eflags);\n \ndiff --git a/grep.h b/grep.h\nindex f6eecc6..24b7d44 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -32,6 +32,7 @@ struct grep_pat {\n \tenum grep_header_field field;\n \tregex_t regexp;\n \tunsigned fixed:1;\n+\tunsigned caseless:1;\n \tunsigned word_regexp:1;\n };\n \n@@ -64,6 +65,7 @@ struct grep_opt {\n \tregex_t regexp;\n \tint linenum;\n \tint invert;\n+\tint caseless;\n \tint status_only;\n \tint name_only;\n \tint unmatch_name_only;\ndiff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\nindex ae5290a..24a9445 100755\n--- a/t/t7002-grep.sh\n+++ b/t/t7002-grep.sh\n@@ -411,4 +411,13 @@ test_expect_success 'grep from a subdirectory to search wider area (2)' '\n \t)\n '\n \n+cat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.6.5.2.187.g29317\n"},{"id":"127690","messageId":"d1c0fbfa0911160858k2cdd35bfvf27df2a5c97348ad@mail.gmail.com","threadId":"21432","inReplyTo":"20091116162533.GA30737@coredump.intra.peff.net","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Brian Collins","fromEmail":"bricollins@gmail.com","sentAt":"2009-11-16T16:58:26Z","receivedAt":"2009-11-16T16:58:26Z","isPatch":true,"sender":{"key":"bricollins@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22232?v=4"},"body":"> 1. The patch was line-wrapped; I had to de-munge it manually to apply.\n\nUgh, does gmail do this? Sorry\n\n> 3. No signed-off-by. Brian, can you please acknowledge the DCO with a\n> signoff?\n\nSigned-off-by: Brian Collins \n\n> 4. The patch introduced some stray trailing whitespace.\n>\n> 5. There were a few style fixups, like omitting braces for a\n> single-line conditional.\n\nAgain sorry, thought the opposite was in the style guide.\n\n--\nBrian Collins\n"},{"id":"127693","messageId":"UENI3LuferIx-aKjVXiKMWqGarArdQQZ0OBA6OkGt_JEiX2iS4H8Wg@cipher.nrlssc.navy.mil","threadId":"21432","inReplyTo":"d1c0fbfa0911160858k2cdd35bfvf27df2a5c97348ad@mail.gmail.com","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2009-11-16T18:00:12Z","receivedAt":"2009-11-16T18:00:12Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"Brian Collins wrote:\n>> 1. The patch was line-wrapped; I had to de-munge it manually to apply.\n> \n> Ugh, does gmail do this?\n\nIn my experience, yes.\n\nThere's a section in Documentation/SubmittingPatches about using the imap\ninterface to upload patches into gmail's \"Drafts\" folder. SubmittingPatches\nthen says to \"Go to your Gmail account, open the Drafts folder, and find the\npatch email, fill in the To: and CC: fields and send away\". I tried that\nonce and the gmail web interface still introduced new lines. Either I'm doing\nsomething wrong, or that document should be changed to clarify that the web\ninterface cannot be used even if the patch is uploaded to the Drafts folder\nvia imap.\n\n-brandon\n"},{"id":"127710","messageId":"7vocn2m48d.fsf@alter.siamese.dyndns.org","threadId":"21432","inReplyTo":"20091116195050.6117@nanako3.lavabit.com","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-16T23:36:50Z","receivedAt":"2009-11-16T23:36:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> Quoting Brian Collins \n>\n>> You will have to excuse me, this is my first patch and I don't know if\n>> this is the right place to post this. Apologies in advance if I'm in\n>> the wrong place.\n>>\n>> git-grep currently throws an error when you combine the -F and -i\n>> flags. This isn't in line with how GNU grep handles it. This patch\n>> allows the simultaneous use of those flags.\n>\n> Junio, may I ask what happened to this patch?\n\nWe got sidetracked into a larger picture issues of how to allow platform\nports to selectively call out to external grep depending on the feature\nset supported by the external grep implementations.\n\nLater I looked at the original patch, the patch text looked fine (except\nthat I would have called the field \"ignorecase\", not \"caseless\"), but it\nwasn't signed off and did not have usable log message.\n\nAnd then I forgot ;-)\n\nThanks for a reminder, and thanks Jeff for a resend.\n"},{"id":"127714","messageId":"7v639am2uq.fsf@alter.siamese.dyndns.org","threadId":"21432","inReplyTo":"7vocn2m48d.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-17T00:06:37Z","receivedAt":"2009-11-17T00:06:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> We got sidetracked into a larger picture issues of how to allow platform\n> ports to selectively call out to external grep depending on the feature\n> set supported by the external grep implementations.\n>\n> Later I looked at the original patch, the patch text looked fine (except\n> that I would have called the field \"ignorecase\", not \"caseless\"), but it\n> wasn't signed off and did not have usable log message.\n>\n> And then I forgot ;-)\n>\n> Thanks for a reminder, and thanks Jeff for a resend.\n\nBy the way, I would suggest updating the test like the attached.\n\nBy looking for rEtUrN, you will catch a bug that breaks \"-i\"-ness\nof the grep, but your test does not catch breakages in \"-F\"-ness.\n\nI am also tempted to add --no-ext-grep to this test, but that would be a\nseparate fix when it becomes necessary, I guess.\n\ndiff --git a/t/t7002-grep.sh b/t/t7002-grep.sh\nindex 87b47dd..35a1e7a 100755\n--- a/t/t7002-grep.sh\n+++ b/t/t7002-grep.sh\n@@ -14,6 +14,7 @@ int main(int argc, const char **argv)\n {\n \tprintf(\"Hello world.\\n\");\n \treturn 0;\n+\t/* char ?? */\n }\n EOF\n \n@@ -346,11 +347,11 @@ test_expect_success 'grep from a subdirectory to search wider area (2)' '\n '\n \n cat >expected <actual &&\n+\tgit grep -Fi \"CHAR *\" >actual &&\n \ttest_cmp expected actual\n '\n \n"},{"id":"127744","messageId":"20091117073845.GC4007@coredump.intra.peff.net","threadId":"21432","inReplyTo":"7v639am2uq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-17T07:38:45Z","receivedAt":"2009-11-17T07:38:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 16, 2009 at 04:06:37PM -0800, Junio C Hamano wrote:\n\n> By the way, I would suggest updating the test like the attached.\n> \n> By looking for rEtUrN, you will catch a bug that breaks \"-i\"-ness\n> of the grep, but your test does not catch breakages in \"-F\"-ness.\n\nYour change looks good to me.\n\n-Peff\n"}]}