On 09.05.2011 18:48:36 -0700 Junio C Hamano <gitster@xxxxxxxxx> wrote:
> MichaÅ Kiedrowicz <michal.kiedrowicz@xxxxxxxxx> 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.
Sounds good, thanks.
>
> We would need some tests for "grep -P", no?
What about those in patch 5/6?
> 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?
If you look at patch 3/6 you will see:
+
+ if (opt->ignore_case)
+ options |= PCRE_CASELESS;
+
+ p->pcre_regexp = pcre_compile(p->pattern, options, &error,
&erroffset,
+ NULL);
and also
+test_expect_success LIBPCRE 'grep -P -i pattern' '
in patch 5/6 :). Or perhaps it doesn't work for you?
>
> 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))
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html