Re: [PATCH v3 6/6] git-grep: Bail out when -P is used with -F or -E

[Junio C Hamano <gitster@pobox.com>]

Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:

> This also makes it bail out when grep.extendedRegexp is enabled.

That is a no-starter.  "git grep -P foo" from the command line should just
ignore the configured default.  It is not entirely your fault, as I think
you inherited the bug from the existing code that lets grep.extendedRegexp
interact with the "--fixed" option from the command line.

> But `git grep -G -P pattern` and `git grep -E -G -P pattern` still work
> because -G and -E set opts.regflags during parse_options() and there is
> no way to detect `-G` or `-E -G`.

How about following the usual pattern of letting the last one win?

Perhaps like this?  This is not even compile tested, but should apply
cleanly on top of, and can be squashed into, your 6/6.  You of course
would need to rewrite the commit log message and documentation, if you
said only one of these can be used.

We would need some tests for "grep -P", no?  Please throw in the "last one
wins" and "command line defeats configuration" when you add one.

Also I deliberately said "--ignore-case and -P are not compatible (yet)";
shouldn't you be able to do ignore case fairly easily, I wonder?  Isn't it
just the matter of wrapping each one with "(?i:" and ")" pair, or anything
more involved necessary?

 builtin/grep.c |   58 +++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 8e422b3..37f2331 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -753,6 +753,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	int i;
 	int dummy;
 	int use_index = 1;
+	enum {
+		pattern_type_unspecified = 0,
+		pattern_type_bre,
+		pattern_type_ere,
+		pattern_type_fixed,
+		pattern_type_pcre,
+	};
+	int pattern_type = pattern_type_unspecified;
+
 	struct option options[] = {
 		OPT_BOOLEAN(0, "cached", &cached,
 			"search in index instead of in the work tree"),
@@ -774,15 +783,18 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			"descend at most <depth> levels", PARSE_OPT_NONEG,
 			NULL, 1 },
 		OPT_GROUP(""),
-		OPT_BIT('E', "extended-regexp", &opt.regflags,
-			"use extended POSIX regular expressions", REG_EXTENDED),
-		OPT_NEGBIT('G', "basic-regexp", &opt.regflags,
-			"use basic POSIX regular expressions (default)",
-			REG_EXTENDED),
-		OPT_BOOLEAN('F', "fixed-strings", &opt.fixed,
-			"interpret patterns as fixed strings"),
-		OPT_BOOLEAN('P', "perl-regexp", &opt.pcre,
-				"use Perl-compatible regular expressions"),
+		OPT_SET_INT('E', "extended-regexp", &pattern_type,
+			    "use extended POSIX regular expressions",
+			    pattern_type_ere),
+		OPT_SET_INT('G', "basic-regexp", &pattern_type,
+			    "use basic POSIX regular expressions (default)",
+			    pattern_type_bre),
+		OPT_SET_INT('F', "fixed-strings", &pattern_type,
+			    "interpret patterns as fixed strings",
+			    pattern_type_fixed),
+		OPT_SET_INT('P', "perl-regexp", &pattern_type,
+			    "use Perl-compatible regular expressions",
+			    pattern_type_pcre),
 		OPT_GROUP(""),
 		OPT_BOOLEAN('n', "line-number", &opt.linenum, "show line numbers"),
 		OPT_NEGBIT('h', NULL, &opt.pathname, "don't show filenames", 1),
@@ -888,6 +900,28 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_KEEP_DASHDASH |
 			     PARSE_OPT_STOP_AT_NON_OPTION |
 			     PARSE_OPT_NO_INTERNAL_HELP);
+	switch (pattern_type) {
+	case pattern_type_fixed:
+		opt.fixed = 1;
+		opt.pcre = 0;
+		break;
+	case pattern_type_bre:
+		opt.fixed = 0;
+		opt.pcre = 0;
+		opt.regflags &= ~REG_EXTENDED;
+		break;
+	case pattern_type_ere:
+		opt.fixed = 0;
+		opt.pcre = 0;
+		opt.regflags |= REG_EXTENDED;
+		break;
+	case pattern_type_pcre:
+		opt.fixed = 0;
+		opt.pcre = 1;
+		break;
+	default:
+		break; /* nothing */
+	}
 
 	if (use_index && !startup_info->have_repository)
 		/* die the same way as if we did it at the beginning */
@@ -925,12 +959,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 
 	if (!opt.pattern_list)
 		die(_("no pattern given."));
-	if (opt.regflags != REG_NEWLINE && opt.pcre)
-		die(_("cannot mix --extended-regexp and --perl-regexp"));
 	if (!opt.fixed && opt.ignore_case)
 		opt.regflags |= REG_ICASE;
-	if ((opt.regflags != REG_NEWLINE || opt.pcre) && opt.fixed)
-		die(_("cannot mix --fixed-strings and regexp"));
+	if (opt.pcre && opt.ignore_case)
+		die(_("--ignore-case and -P are not compatible (yet)"));
 
 #ifndef NO_PTHREADS
 	if (online_cpus() == 1 || !grep_threads_ok(&opt))