[PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Brian Collins]

You will have to excuse me, this is my first patch and I don't know if  
this is the right place to post this. Apologies in advance if I'm in  
the wrong place.

git-grep currently throws an error when you combine the -F and -i  
flags. This isn't in line with how GNU grep handles it. This patch  
allows the simultaneous use of those flags.

---
  builtin-grep.c |    8 +++++---
  grep.c         |   16 ++++++++++++----
  grep.h         |    2 ++
  3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 761799d..c73f05b 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -367,7 +367,7 @@ static int external_grep(struct grep_opt *opt,  
const char **paths, int cached)
  		push_arg("-h");
  	if (opt->regflags & REG_EXTENDED)
  		push_arg("-E");
-	if (opt->regflags & REG_ICASE)
+	if (opt->caseless)
  		push_arg("-i");
  	if (opt->binary == GREP_BINARY_NOMATCH)
  		push_arg("-I");
@@ -706,8 +706,8 @@ int cmd_grep(int argc, const char **argv, const  
char *prefix)
  		OPT_GROUP(""),
  		OPT_BOOLEAN('v', "invert-match", &opt.invert,
  			"show non-matching lines"),
-		OPT_BIT('i', "ignore-case", &opt.regflags,
-			"case insensitive matching", REG_ICASE),
+		OPT_BOOLEAN('i', "ignore-case", &opt.caseless,
+			"case insensitive matching"),
  		OPT_BOOLEAN('w', "word-regexp", &opt.word_regexp,
  			"match patterns only at word boundaries"),
  		OPT_SET_INT('a', "text", &opt.binary,
@@ -830,6 +830,8 @@ int cmd_grep(int argc, const char **argv, const  
char *prefix)
  		external_grep_allowed = 0;
  	if (!opt.pattern_list)
  		die("no pattern given.");
+	if (!opt.fixed && opt.caseless)
+		opt.regflags |= REG_ICASE;
  	if ((opt.regflags != REG_NEWLINE) && opt.fixed)
  		die("cannot mix --fixed-strings and regexp");
  	compile_grep_patterns(&opt);
diff --git a/grep.c b/grep.c
index 5d162da..d8f14be 100644
--- a/grep.c
+++ b/grep.c
@@ -41,7 +41,8 @@ static void compile_regexp(struct grep_pat *p,  
struct grep_opt *opt)
  	int err;

  	p->word_regexp = opt->word_regexp;
-
+	p->caseless = opt->caseless;
+	
  	if (opt->fixed || is_fixed(p->pattern))
  		p->fixed = 1;
  	if (opt->regflags & REG_ICASE)
@@ -262,9 +263,16 @@ static void show_name(struct grep_opt *opt, const  
char *name)
  	printf("%s%c", name, opt->null_following_name ? '\0' : '\n');
  }

-static int fixmatch(const char *pattern, char *line, regmatch_t *match)
+
+static int fixmatch(const char *pattern, char *line, int caseless,  
regmatch_t *match)
  {
-	char *hit = strstr(line, pattern);
+	char *hit;
+	if (caseless) {
+		hit = strcasestr(line, pattern);
+	} else {
+		hit = strstr(line, pattern);
+	}
+	
  	if (!hit) {
  		match->rm_so = match->rm_eo = -1;
  		return REG_NOMATCH;
@@ -326,7 +334,7 @@ static int match_one_pattern(struct grep_pat *p,  
char *bol, char *eol,

   again:
  	if (p->fixed)
-		hit = !fixmatch(p->pattern, bol, pmatch);
+		hit = !fixmatch(p->pattern, bol, p->caseless, pmatch);
  	else
  		hit = !regexec(&p->regexp, bol, 1, pmatch, eflags);

diff --git a/grep.h b/grep.h
index f6eecc6..24b7d44 100644
--- a/grep.h
+++ b/grep.h
@@ -32,6 +32,7 @@ struct grep_pat {
  	enum grep_header_field field;
  	regex_t regexp;
  	unsigned fixed:1;
+	unsigned caseless:1;
  	unsigned word_regexp:1;
  };

@@ -64,6 +65,7 @@ struct grep_opt {
  	regex_t regexp;
  	int linenum;
  	int invert;
+	int caseless;
  	int status_only;
  	int name_only;
  	int unmatch_name_only;
-- 
1.6.4.4

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Jeff King]

On Thu, Oct 29, 2009 at 06:21:59PM -0700, Brian Collins wrote:

> You will have to excuse me, this is my first patch and I don't know
> if this is the right place to post this. Apologies in advance if I'm
> in the wrong place.

You're in the right place (though judging from the response, nobody
seemed to find your patch all that interesting...).

> git-grep currently throws an error when you combine the -F and -i
> flags. This isn't in line with how GNU grep handles it. This patch
> allows the simultaneous use of those flags.

I don't see a reason not to allow this combination if our grep
implementation supports it. My only reservation would be that we
sometimes call out to an external grep, and non-GNU grep might barf on
this. But I think that is OK, as the user should get a sane error from
the external grep.

>  builtin-grep.c |    8 +++++---
>  grep.c         |   16 ++++++++++++----
>  grep.h         |    2 ++

Tests? They help prove to us that your feature works, and also prevent
us from accidentally breaking your feature in the future.

-Peff

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Brian Collins]

2009/11/6 Jeff King <peff@peff.net>:
> You're in the right place (though judging from the response, nobody
> seemed to find your patch all that interesting...).

Heh, yeah it is a bit of a boring edge case but a TextMate plugin I am writing
requires this functionality.

> Tests? They help prove to us that your feature works, and also prevent
> us from accidentally breaking your feature in the future.

Ah yes that is what I was forgetting. Please see the amended patch including
test.
Thanks for your help


---
 builtin-grep.c  |    8 +++++---
 grep.c          |   16 ++++++++++++----
 grep.h          |    2 ++
 t/t7002-grep.sh |    9 +++++++++
 4 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 761799d..c73f05b 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -367,7 +367,7 @@ static int external_grep(struct grep_opt *opt,
const char **paths, int cached)
 		push_arg("-h");
 	if (opt->regflags & REG_EXTENDED)
 		push_arg("-E");
-	if (opt->regflags & REG_ICASE)
+	if (opt->caseless)
 		push_arg("-i");
 	if (opt->binary == GREP_BINARY_NOMATCH)
 		push_arg("-I");
@@ -706,8 +706,8 @@ int cmd_grep(int argc, const char **argv, const
char *prefix)
 		OPT_GROUP(""),
 		OPT_BOOLEAN('v', "invert-match", &opt.invert,
 			"show non-matching lines"),
-		OPT_BIT('i', "ignore-case", &opt.regflags,
-			"case insensitive matching", REG_ICASE),
+		OPT_BOOLEAN('i', "ignore-case", &opt.caseless,
+			"case insensitive matching"),
 		OPT_BOOLEAN('w', "word-regexp", &opt.word_regexp,
 			"match patterns only at word boundaries"),
 		OPT_SET_INT('a', "text", &opt.binary,
@@ -830,6 +830,8 @@ int cmd_grep(int argc, const char **argv, const
char *prefix)
 		external_grep_allowed = 0;
 	if (!opt.pattern_list)
 		die("no pattern given.");
+	if (!opt.fixed && opt.caseless)
+		opt.regflags |= REG_ICASE;
 	if ((opt.regflags != REG_NEWLINE) && opt.fixed)
 		die("cannot mix --fixed-strings and regexp");
 	compile_grep_patterns(&opt);
diff --git a/grep.c b/grep.c
index 5d162da..d8f14be 100644
--- a/grep.c
+++ b/grep.c
@@ -41,7 +41,8 @@ static void compile_regexp(struct grep_pat *p,
struct grep_opt *opt)
 	int err;

 	p->word_regexp = opt->word_regexp;
-
+	p->caseless = opt->caseless;
+	
 	if (opt->fixed || is_fixed(p->pattern))
 		p->fixed = 1;
 	if (opt->regflags & REG_ICASE)
@@ -262,9 +263,16 @@ static void show_name(struct grep_opt *opt, const
char *name)
 	printf("%s%c", name, opt->null_following_name ? '\0' : '\n');
 }

-static int fixmatch(const char *pattern, char *line, regmatch_t *match)
+
+static int fixmatch(const char *pattern, char *line, int caseless,
regmatch_t *match)
 {
-	char *hit = strstr(line, pattern);
+	char *hit;
+	if (caseless) {
+		hit = strcasestr(line, pattern);
+	} else {
+		hit = strstr(line, pattern);
+	}
+	
 	if (!hit) {
 		match->rm_so = match->rm_eo = -1;
 		return REG_NOMATCH;
@@ -326,7 +334,7 @@ static int match_one_pattern(struct grep_pat *p,
char *bol, char *eol,

  again:
 	if (p->fixed)
-		hit = !fixmatch(p->pattern, bol, pmatch);
+		hit = !fixmatch(p->pattern, bol, p->caseless, pmatch);
 	else
 		hit = !regexec(&p->regexp, bol, 1, pmatch, eflags);

diff --git a/grep.h b/grep.h
index f6eecc6..24b7d44 100644
--- a/grep.h
+++ b/grep.h
@@ -32,6 +32,7 @@ struct grep_pat {
 	enum grep_header_field field;
 	regex_t regexp;
 	unsigned fixed:1;
+	unsigned caseless:1;
 	unsigned word_regexp:1;
 };

@@ -64,6 +65,7 @@ struct grep_opt {
 	regex_t regexp;
 	int linenum;
 	int invert;
+	int caseless;
 	int status_only;
 	int name_only;
 	int unmatch_name_only;
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index ae56a36..87b47dd 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -345,4 +345,13 @@ test_expect_success 'grep from a subdirectory to
search wider area (2)' '
 	)
 '

+cat >expected <<EOF
+hello.c:	return 0;
+EOF
+
+test_expect_success 'grep -Fi' '
+	git grep -Fi rEtUrN >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.6.4.4

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> git-grep currently throws an error when you combine the -F and -i
>> flags. This isn't in line with how GNU grep handles it. This patch
>> allows the simultaneous use of those flags.
>
> I don't see a reason not to allow this combination if our grep
> implementation supports it. My only reservation would be that we
> sometimes call out to an external grep, and non-GNU grep might barf on
> this. But I think that is OK, as the user should get a sane error from
> the external grep.

I think that is OK but not for the reason you stated.  The user should
never get such an error message.

The reason why I think this is OK is because the builder can choose to use
NO_EXTERNAL_GREP if the external grep does not allow this combination.

We need to update comments on the Makefile if we are going to take this
patch.  Currently the description suggests three possible reasons you
might want to choose NO_EXTERNAL_GREP.  "lacks grep" is obvious, "slower"
is not about correctness, but "is broken" is way underspecified, and this
patch adds one more reason to label your "grep" as broken.

Currently, SunOS and IRIX/IRIX64 are the only ones that specify
NO_EXTERNAL_GREP, and I suspect both are defined due to "is broken", but
we do not tell the builder in what way they are considered broken, iow,
what features we expect from the platform "grep", so somebody coming up
with a new port would be at loss.

I suspect 01ae841 (SunOS grep does not understand -C<n> nor -e, 2009-07-23)
would be a good starting point.  Something like this...

    # Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
    # your external grep (e.g., if your system lacks grep, if its grep
    # does not support necessary features, or spawning external process is
    # slower than built-in grep git has).  To be usable, your grep must
    # support -C<n> (show n lines of context), -e <pattern> (primarily
    # used to quote a pattern that begins with a dash), and use of -F and
    # -i at the same time.  Otherwise define this.

But I didn't try hard to find out what _else_ we are depending on.

> Tests? They help prove to us that your feature works, and also prevent
> us from accidentally breaking your feature in the future.

Yeah, thanks.  The latter is the primary purpose of test.

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Jeff King]

On Fri, Nov 06, 2009 at 02:00:11AM -0800, Junio C Hamano wrote:

> > I don't see a reason not to allow this combination if our grep
> > implementation supports it. My only reservation would be that we
> > sometimes call out to an external grep, and non-GNU grep might barf on
> > this. But I think that is OK, as the user should get a sane error from
> > the external grep.
> 
> I think that is OK but not for the reason you stated.  The user should
> never get such an error message.
> 
> The reason why I think this is OK is because the builder can choose to use
> NO_EXTERNAL_GREP if the external grep does not allow this combination.

Yes, I think that would be a sane thing to do (and I suspect anyone
using non-GNU grep is probably already doing it). But what I meant more
was that the _transition_ should be fine. If we start shipping with this
patch but people haven't updated their build configuration, it is not
going to break horribly; it should just produce an error, which is what
it is doing now.

>     # Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
>     # your external grep (e.g., if your system lacks grep, if its grep
>     # does not support necessary features, or spawning external process is
>     # slower than built-in grep git has).  To be usable, your grep must
>     # support -C<n> (show n lines of context), -e <pattern> (primarily
>     # used to quote a pattern that begins with a dash), and use of -F and
>     # -i at the same time.  Otherwise define this.
> 
> But I didn't try hard to find out what _else_ we are depending on.

It is not really _us_ depending on it. It is "things the user wants to
do that _we_ support, but that their grep might not." So I don't think
there is much point in enumerating features. If their system grep
doesn't handle options that they want to use, then it won't work for
them. If they don't use them, then they will be fine.

Though "-e" might be the exception, as I think we might use it
unconditionally. But something like "-F -i" really depends on whether
the user wants to use it.

So I am fine with the text above, but I wouldn't worry too hard about
trying to come up with an exhaustive feature list.

-Peff

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Nov 06, 2009 at 02:00:11AM -0800, Junio C Hamano wrote:
>
>> But I didn't try hard to find out what _else_ we are depending on.
>
> It is not really _us_ depending on it. It is "things the user wants to
> do that _we_ support, but that their grep might not." So I don't think
> there is much point in enumerating features. If their system grep
> doesn't handle options that they want to use, then it won't work for
> them. If they don't use them, then they will be fine.
>
> Though "-e" might be the exception, as I think we might use it
> unconditionally. But something like "-F -i" really depends on whether
> the user wants to use it.

Yes and no.

Even though we currently punt on a few platforms for simplicity and build
with NO_EXTERNAL_GREP, we could check if the set of options given are
within the feature set of what the platform's grep understands and choose
to spawn "grep" unless some options that are unsupported are used, in
which case we fall back to the internal one.

We could certainly do something like this if it turns out to be a problem.
An invocation that does not use -F and -i together can still spawn
external grep if that is faster.

You are correct about "-e".  Our NO_EXTERNAL_GREP on SunOS cannot be
avoided.

 builtin-grep.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 1df25b0..2905f64 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -357,6 +357,9 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 
 	if (opt->extended || (opt->relative && opt->prefix_length))
 		return -1;
+	if (NO_GREP_FIXED_IGNORE_CASE &&
+	    opt->fixed && (opt->regflags & REG_ICASE))
+		return -1;
 	len = nr = 0;
 	push_arg("grep");
 	if (opt->fixed)

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Nanako Shiraishi]

Quoting Brian Collins <bricollins@gmail.com>

> You will have to excuse me, this is my first patch and I don't know if
> this is the right place to post this. Apologies in advance if I'm in
> the wrong place.
>
> git-grep currently throws an error when you combine the -F and -i
> flags. This isn't in line with how GNU grep handles it. This patch
> allows the simultaneous use of those flags.

Junio, may I ask what happened to this patch?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Jeff King]

On Mon, Nov 16, 2009 at 07:50:50PM +0900, Nanako Shiraishi wrote:

> Quoting Brian Collins <bricollins@gmail.com>
> 
> > You will have to excuse me, this is my first patch and I don't know if
> > this is the right place to post this. Apologies in advance if I'm in
> > the wrong place.
> >
> > git-grep currently throws an error when you combine the -F and -i
> > flags. This isn't in line with how GNU grep handles it. This patch
> > allows the simultaneous use of those flags.
> 
> Junio, may I ask what happened to this patch?

I think I owed it another review.

I just looked it over and the idea and implementation look sane to me.
There were a few minor problems with the submission:

  1. The patch was line-wrapped; I had to de-munge it manually to apply.

  2. The original submission had cover-letter material mixed in with the
     commit message. The follow-up version had no commit message at all.

  3. No signed-off-by. Brian, can you please acknowledge the DCO with a
     signoff?

  4. The patch introduced some stray trailing whitespace.

  5. There were a few style fixups, like omitting braces for a
     single-line conditional.

To save Junio time, here is a version that fixes all of those things. I
think it's probably worth applying to 'next'.

-- >8 --
From: Brian Collins <bricollins@gmail.com>
Date: Fri, 6 Nov 2009 01:22:35 -0800
Subject: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

git-grep currently throws an error when you combine the -F
and -i flags.  This isn't in line with how GNU grep handles
it. This patch allows the simultaneous use of those flags.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-grep.c  |    8 +++++---
 grep.c          |   13 ++++++++++---
 grep.h          |    2 ++
 t/t7002-grep.sh |    9 +++++++++
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 1df25b0..a2616fc 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -367,7 +367,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 		push_arg("-h");
 	if (opt->regflags & REG_EXTENDED)
 		push_arg("-E");
-	if (opt->regflags & REG_ICASE)
+	if (opt->caseless)
 		push_arg("-i");
 	if (opt->binary == GREP_BINARY_NOMATCH)
 		push_arg("-I");
@@ -706,8 +706,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_BOOLEAN('v', "invert-match", &opt.invert,
 			"show non-matching lines"),
-		OPT_BIT('i', "ignore-case", &opt.regflags,
-			"case insensitive matching", REG_ICASE),
+		OPT_BOOLEAN('i', "ignore-case", &opt.caseless,
+			"case insensitive matching"),
 		OPT_BOOLEAN('w', "word-regexp", &opt.word_regexp,
 			"match patterns only at word boundaries"),
 		OPT_SET_INT('a', "text", &opt.binary,
@@ -830,6 +830,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		external_grep_allowed = 0;
 	if (!opt.pattern_list)
 		die("no pattern given.");
+	if (!opt.fixed && opt.caseless)
+		opt.regflags |= REG_ICASE;
 	if ((opt.regflags != REG_NEWLINE) && opt.fixed)
 		die("cannot mix --fixed-strings and regexp");
 	compile_grep_patterns(&opt);
diff --git a/grep.c b/grep.c
index 5d162da..bbb0d18 100644
--- a/grep.c
+++ b/grep.c
@@ -41,6 +41,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
 	int err;
 
 	p->word_regexp = opt->word_regexp;
+	p->caseless = opt->caseless;
 
 	if (opt->fixed || is_fixed(p->pattern))
 		p->fixed = 1;
@@ -262,9 +263,15 @@ static void show_name(struct grep_opt *opt, const char *name)
 	printf("%s%c", name, opt->null_following_name ? '\0' : '\n');
 }
 
-static int fixmatch(const char *pattern, char *line, regmatch_t *match)
+
+static int fixmatch(const char *pattern, char *line, int caseless, regmatch_t *match)
 {
-	char *hit = strstr(line, pattern);
+	char *hit;
+	if (caseless)
+		hit = strcasestr(line, pattern);
+	else
+		hit = strstr(line, pattern);
+
 	if (!hit) {
 		match->rm_so = match->rm_eo = -1;
 		return REG_NOMATCH;
@@ -326,7 +333,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 
  again:
 	if (p->fixed)
-		hit = !fixmatch(p->pattern, bol, pmatch);
+		hit = !fixmatch(p->pattern, bol, p->caseless, pmatch);
 	else
 		hit = !regexec(&p->regexp, bol, 1, pmatch, eflags);
 
diff --git a/grep.h b/grep.h
index f6eecc6..24b7d44 100644
--- a/grep.h
+++ b/grep.h
@@ -32,6 +32,7 @@ struct grep_pat {
 	enum grep_header_field field;
 	regex_t regexp;
 	unsigned fixed:1;
+	unsigned caseless:1;
 	unsigned word_regexp:1;
 };
 
@@ -64,6 +65,7 @@ struct grep_opt {
 	regex_t regexp;
 	int linenum;
 	int invert;
+	int caseless;
 	int status_only;
 	int name_only;
 	int unmatch_name_only;
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index ae5290a..24a9445 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -411,4 +411,13 @@ test_expect_success 'grep from a subdirectory to search wider area (2)' '
 	)
 '
 
+cat >expected <<EOF
+hello.c:	return 0;
+EOF
+
+test_expect_success 'grep -Fi' '
+	git grep -Fi rEtUrN >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.6.5.2.187.g29317

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Brian Collins]

>  1. The patch was line-wrapped; I had to de-munge it manually to apply.

Ugh, does gmail do this? Sorry

>  3. No signed-off-by. Brian, can you please acknowledge the DCO with a
>     signoff?

Signed-off-by: Brian Collins <bricollins@gmail.com>

>  4. The patch introduced some stray trailing whitespace.
>
>  5. There were a few style fixups, like omitting braces for a
>     single-line conditional.

Again sorry, thought the opposite was in the style guide.

--
Brian Collins

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Brandon Casey]

Brian Collins wrote:
>>  1. The patch was line-wrapped; I had to de-munge it manually to apply.
> 
> Ugh, does gmail do this?

In my experience, yes.

There's a section in Documentation/SubmittingPatches about using the imap
interface to upload patches into gmail's "Drafts" folder.  SubmittingPatches
then says to "Go to your Gmail account, open the Drafts folder, and find the
patch email, fill in the To: and CC: fields and send away".  I tried that
once and the gmail web interface still introduced new lines.  Either I'm doing
something wrong, or that document should be changed to clarify that the web
interface cannot be used even if the patch is uploaded to the Drafts folder
via imap.

-brandon

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting Brian Collins <bricollins@gmail.com>
>
>> You will have to excuse me, this is my first patch and I don't know if
>> this is the right place to post this. Apologies in advance if I'm in
>> the wrong place.
>>
>> git-grep currently throws an error when you combine the -F and -i
>> flags. This isn't in line with how GNU grep handles it. This patch
>> allows the simultaneous use of those flags.
>
> Junio, may I ask what happened to this patch?

We got sidetracked into a larger picture issues of how to allow platform
ports to selectively call out to external grep depending on the feature
set supported by the external grep implementations.

Later I looked at the original patch, the patch text looked fine (except
that I would have called the field "ignorecase", not "caseless"), but it
wasn't signed off and did not have usable log message.

And then I forgot ;-)

Thanks for a reminder, and thanks Jeff for a resend.

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> We got sidetracked into a larger picture issues of how to allow platform
> ports to selectively call out to external grep depending on the feature
> set supported by the external grep implementations.
>
> Later I looked at the original patch, the patch text looked fine (except
> that I would have called the field "ignorecase", not "caseless"), but it
> wasn't signed off and did not have usable log message.
>
> And then I forgot ;-)
>
> Thanks for a reminder, and thanks Jeff for a resend.

By the way, I would suggest updating the test like the attached.

By looking for rEtUrN, you will catch a bug that breaks "-i"-ness
of the grep, but your test does not catch breakages in "-F"-ness.

I am also tempted to add --no-ext-grep to this test, but that would be a
separate fix when it becomes necessary, I guess.

diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index 87b47dd..35a1e7a 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -14,6 +14,7 @@ int main(int argc, const char **argv)
 {
 	printf("Hello world.\n");
 	return 0;
+	/* char ?? */
 }
 EOF
 
@@ -346,11 +347,11 @@ test_expect_success 'grep from a subdirectory to search wider area (2)' '
 '
 
 cat >expected <<EOF
-hello.c:	return 0;
+hello.c:int main(int argc, const char **argv)
 EOF
 
 test_expect_success 'grep -Fi' '
-	git grep -Fi rEtUrN >actual &&
+	git grep -Fi "CHAR *" >actual &&
 	test_cmp expected actual
 '
 

Re: [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings

[Jeff King]

On Mon, Nov 16, 2009 at 04:06:37PM -0800, Junio C Hamano wrote:

> By the way, I would suggest updating the test like the attached.
> 
> By looking for rEtUrN, you will catch a bug that breaks "-i"-ness
> of the grep, but your test does not catch breakages in "-F"-ness.

Your change looks good to me.

-Peff