Merge branch 'jc/grep-pcre-loose-ends'

"git log -F -E --grep='<ere>'" failed to use the given <ere>
pattern as extended regular expression, and instead looked for the
string literally.  The early part of this series is a fix for it;
the latter part teaches log to respect the grep.* configuration.

* jc/grep-pcre-loose-ends:
log: honor grep.* configuration
log --grep: accept --basic-regexp and --perl-regexp
log --grep: use the same helper to set -E/-F options as "git grep"
revisions: initialize revs->grep_filter using grep_init()
grep: move pattern-type bits support to top-level grep.[ch]
grep: move the configuration parsing logic to grep.[ch]
builtin/grep.c: make configuration callback more reusable

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index ee497430cb0f8dc0859b90b8773e6b8e28d5e361..1ec14a068e750b114ac5f1ad2f468de8d0664a16 100644 (file)
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -79,6 +79,11 @@ if it is part of the log message.
 
        Match the regexp limiting patterns without regard to letters case.
 
+--basic-regexp::
+
+       Consider the limiting patterns to be basic regular expressions;
+       this is the default.
+
 -E::
 --extended-regexp::
 
@@ -91,6 +96,11 @@ if it is part of the log message.
        Consider the limiting patterns to be fixed strings (don't interpret
        pattern as a regular expression).
 
+--perl-regexp::
+
+       Consider the limiting patterns to be Perl-compatible regexp.
+       Requires libpcre to be compiled in.
+
 --remove-empty::
 
        Stop when a given path disappears from the tree.

diff --git a/builtin/grep.c b/builtin/grep.c
index 82530a61b483738382c810bcb141e33b06729d6f..c296e6f99e0c96807c2d95f3e396d01878e1f04b 100644 (file)
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -261,103 +261,12 @@ static int wait_all(void)
 }
 #endif
 
-static int parse_pattern_type_arg(const char *opt, const char *arg)
+static int grep_cmd_config(const char *var, const char *value, void *cb)
 {
-       if (!strcmp(arg, "default"))
-               return GREP_PATTERN_TYPE_UNSPECIFIED;
-       else if (!strcmp(arg, "basic"))
-               return GREP_PATTERN_TYPE_BRE;
-       else if (!strcmp(arg, "extended"))
-               return GREP_PATTERN_TYPE_ERE;
-       else if (!strcmp(arg, "fixed"))
-               return GREP_PATTERN_TYPE_FIXED;
-       else if (!strcmp(arg, "perl"))
-               return GREP_PATTERN_TYPE_PCRE;
-       die("bad %s argument: %s", opt, arg);
-}
-
-static void grep_pattern_type_options(const int pattern_type, struct grep_opt *opt)
-{
-       switch (pattern_type) {
-       case GREP_PATTERN_TYPE_UNSPECIFIED:
-               /* fall through */
-
-       case GREP_PATTERN_TYPE_BRE:
-               opt->fixed = 0;
-               opt->pcre = 0;
-               opt->regflags &= ~REG_EXTENDED;
-               break;
-
-       case GREP_PATTERN_TYPE_ERE:
-               opt->fixed = 0;
-               opt->pcre = 0;
-               opt->regflags |= REG_EXTENDED;
-               break;
-
-       case GREP_PATTERN_TYPE_FIXED:
-               opt->fixed = 1;
-               opt->pcre = 0;
-               opt->regflags &= ~REG_EXTENDED;
-               break;
-
-       case GREP_PATTERN_TYPE_PCRE:
-               opt->fixed = 0;
-               opt->pcre = 1;
-               opt->regflags &= ~REG_EXTENDED;
-               break;
-       }
-}
-
-static int grep_config(const char *var, const char *value, void *cb)
-{
-       struct grep_opt *opt = cb;
-       char *color = NULL;
-
-       if (userdiff_config(var, value) < 0)
-               return -1;
-
-       if (!strcmp(var, "grep.extendedregexp")) {
-               if (git_config_bool(var, value))
-                       opt->extended_regexp_option = 1;
-               else
-                       opt->extended_regexp_option = 0;
-               return 0;
-       }
-
-       if (!strcmp(var, "grep.patterntype")) {
-               opt->pattern_type_option = parse_pattern_type_arg(var, value);
-               return 0;
-  }
-
-       if (!strcmp(var, "grep.linenumber")) {
-               opt->linenum = git_config_bool(var, value);
-               return 0;
-       }
-
-       if (!strcmp(var, "color.grep"))
-               opt->color = git_config_colorbool(var, value);
-       else if (!strcmp(var, "color.grep.context"))
-               color = opt->color_context;
-       else if (!strcmp(var, "color.grep.filename"))
-               color = opt->color_filename;
-       else if (!strcmp(var, "color.grep.function"))
-               color = opt->color_function;
-       else if (!strcmp(var, "color.grep.linenumber"))
-               color = opt->color_lineno;
-       else if (!strcmp(var, "color.grep.match"))
-               color = opt->color_match;
-       else if (!strcmp(var, "color.grep.selected"))
-               color = opt->color_selected;
-       else if (!strcmp(var, "color.grep.separator"))
-               color = opt->color_sep;
-       else
-               return git_color_default_config(var, value, cb);
-       if (color) {
-               if (!value)
-                       return config_error_nonbool(var);
-               color_parse(value, var, color);
-       }
-       return 0;
+       int st = grep_config(var, value, cb);
+       if (git_color_default_config(var, value, cb) < 0)
+               st = -1;
+       return st;
 }
 
 static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)
@@ -839,27 +748,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
        if (argc == 2 && !strcmp(argv[1], "-h"))
                usage_with_options(grep_usage, options);
 
-       memset(&opt, 0, sizeof(opt));
-       opt.prefix = prefix;
-       opt.prefix_length = (prefix && *prefix) ? strlen(prefix) : 0;
-       opt.relative = 1;
-       opt.pathname = 1;
-       opt.pattern_tail = &opt.pattern_list;
-       opt.header_tail = &opt.header_list;
-       opt.regflags = REG_NEWLINE;
-       opt.max_depth = -1;
-       opt.pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED;
-       opt.extended_regexp_option = 0;
-
-       strcpy(opt.color_context, "");
-       strcpy(opt.color_filename, "");
-       strcpy(opt.color_function, "");
-       strcpy(opt.color_lineno, "");
-       strcpy(opt.color_match, GIT_COLOR_BOLD_RED);
-       strcpy(opt.color_selected, "");
-       strcpy(opt.color_sep, GIT_COLOR_CYAN);
-       opt.color = -1;
-       git_config(grep_config, &opt);
+       init_grep_defaults();
+       git_config(grep_cmd_config, NULL);
+       grep_init(&opt, prefix);
 
        /*
         * If there is no -- then the paths must exist in the working
@@ -875,13 +766,7 @@ 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);
-
-       if (pattern_type_arg != GREP_PATTERN_TYPE_UNSPECIFIED)
-               grep_pattern_type_options(pattern_type_arg, &opt);
-       else if (opt.pattern_type_option != GREP_PATTERN_TYPE_UNSPECIFIED)
-               grep_pattern_type_options(opt.pattern_type_option, &opt);
-       else if (opt.extended_regexp_option)
-               grep_pattern_type_options(GREP_PATTERN_TYPE_ERE, &opt);
+       grep_commit_pattern_type(pattern_type_arg, &opt);
 
        if (use_index && !startup_info->have_repository)
                /* die the same way as if we did it at the beginning */

diff --git a/builtin/log.c b/builtin/log.c
index 09cf43e6d40efc13f6b7bda2094fca5bdc1bbc87..e7b7db1cacf1934a9bb0b96beb98ddda15a66074 100644 (file)
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -351,7 +351,8 @@ static int git_log_config(const char *var, const char *value, void *cb)
        }
        if (!prefixcmp(var, "color.decorate."))
                return parse_decorate_color_config(var, 15, value);
-
+       if (grep_config(var, value, cb) < 0)
+               return -1;
        return git_diff_ui_config(var, value, cb);
 }
 
@@ -360,6 +361,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
        struct rev_info rev;
        struct setup_revision_opt opt;
 
+       init_grep_defaults();
        git_config(git_log_config, NULL);
 
        init_revisions(&rev, prefix);
@@ -450,6 +452,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
        struct pathspec match_all;
        int i, count, ret = 0;
 
+       init_grep_defaults();
        git_config(git_log_config, NULL);
 
        init_pathspec(&match_all, NULL);
@@ -530,6 +533,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
        struct rev_info rev;
        struct setup_revision_opt opt;
 
+       init_grep_defaults();
        git_config(git_log_config, NULL);
 
        init_revisions(&rev, prefix);
@@ -552,6 +556,7 @@ int cmd_log(int argc, const char **argv, const char *prefix)
        struct rev_info rev;
        struct setup_revision_opt opt;
 
+       init_grep_defaults();
        git_config(git_log_config, NULL);
 
        init_revisions(&rev, prefix);
@@ -1121,6 +1126,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
        extra_hdr.strdup_strings = 1;
        extra_to.strdup_strings = 1;
        extra_cc.strdup_strings = 1;
+       init_grep_defaults();
        git_config(git_format_config, NULL);
        init_revisions(&rev, prefix);
        rev.commit_format = CMIT_FMT_EMAIL;

diff --git a/grep.c b/grep.c
index edc7776677ed201dbe117c207a680e1f832737e3..a947a68a7f26d999db02fa86ccbaa3def8cf78b4 100644 (file)
--- a/grep.c
+++ b/grep.c
@@ -6,6 +6,183 @@
 static int grep_source_load(struct grep_source *gs);
 static int grep_source_is_binary(struct grep_source *gs);
 
+static struct grep_opt grep_defaults;
+
+/*
+ * Initialize the grep_defaults template with hardcoded defaults.
+ * We could let the compiler do this, but without C99 initializers
+ * the code gets unwieldy and unreadable, so...
+ */
+void init_grep_defaults(void)
+{
+       struct grep_opt *opt = &grep_defaults;
+       static int run_once;
+
+       if (run_once)
+               return;
+       run_once++;
+
+       memset(opt, 0, sizeof(*opt));
+       opt->relative = 1;
+       opt->pathname = 1;
+       opt->regflags = REG_NEWLINE;
+       opt->max_depth = -1;
+       opt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED;
+       opt->extended_regexp_option = 0;
+       strcpy(opt->color_context, "");
+       strcpy(opt->color_filename, "");
+       strcpy(opt->color_function, "");
+       strcpy(opt->color_lineno, "");
+       strcpy(opt->color_match, GIT_COLOR_BOLD_RED);
+       strcpy(opt->color_selected, "");
+       strcpy(opt->color_sep, GIT_COLOR_CYAN);
+       opt->color = -1;
+}
+
+static int parse_pattern_type_arg(const char *opt, const char *arg)
+{
+       if (!strcmp(arg, "default"))
+               return GREP_PATTERN_TYPE_UNSPECIFIED;
+       else if (!strcmp(arg, "basic"))
+               return GREP_PATTERN_TYPE_BRE;
+       else if (!strcmp(arg, "extended"))
+               return GREP_PATTERN_TYPE_ERE;
+       else if (!strcmp(arg, "fixed"))
+               return GREP_PATTERN_TYPE_FIXED;
+       else if (!strcmp(arg, "perl"))
+               return GREP_PATTERN_TYPE_PCRE;
+       die("bad %s argument: %s", opt, arg);
+}
+
+/*
+ * Read the configuration file once and store it in
+ * the grep_defaults template.
+ */
+int grep_config(const char *var, const char *value, void *cb)
+{
+       struct grep_opt *opt = &grep_defaults;
+       char *color = NULL;
+
+       if (userdiff_config(var, value) < 0)
+               return -1;
+
+       if (!strcmp(var, "grep.extendedregexp")) {
+               if (git_config_bool(var, value))
+                       opt->extended_regexp_option = 1;
+               else
+                       opt->extended_regexp_option = 0;
+               return 0;
+       }
+
+       if (!strcmp(var, "grep.patterntype")) {
+               opt->pattern_type_option = parse_pattern_type_arg(var, value);
+               return 0;
+       }
+
+       if (!strcmp(var, "grep.linenumber")) {
+               opt->linenum = git_config_bool(var, value);
+               return 0;
+       }
+
+       if (!strcmp(var, "color.grep"))
+               opt->color = git_config_colorbool(var, value);
+       else if (!strcmp(var, "color.grep.context"))
+               color = opt->color_context;
+       else if (!strcmp(var, "color.grep.filename"))
+               color = opt->color_filename;
+       else if (!strcmp(var, "color.grep.function"))
+               color = opt->color_function;
+       else if (!strcmp(var, "color.grep.linenumber"))
+               color = opt->color_lineno;
+       else if (!strcmp(var, "color.grep.match"))
+               color = opt->color_match;
+       else if (!strcmp(var, "color.grep.selected"))
+               color = opt->color_selected;
+       else if (!strcmp(var, "color.grep.separator"))
+               color = opt->color_sep;
+
+       if (color) {
+               if (!value)
+                       return config_error_nonbool(var);
+               color_parse(value, var, color);
+       }
+       return 0;
+}
+
+/*
+ * Initialize one instance of grep_opt and copy the
+ * default values from the template we read the configuration
+ * information in an earlier call to git_config(grep_config).
+ */
+void grep_init(struct grep_opt *opt, const char *prefix)
+{
+       struct grep_opt *def = &grep_defaults;
+
+       memset(opt, 0, sizeof(*opt));
+       opt->prefix = prefix;
+       opt->prefix_length = (prefix && *prefix) ? strlen(prefix) : 0;
+       opt->pattern_tail = &opt->pattern_list;
+       opt->header_tail = &opt->header_list;
+
+       opt->color = def->color;
+       opt->extended_regexp_option = def->extended_regexp_option;
+       opt->pattern_type_option = def->pattern_type_option;
+       opt->linenum = def->linenum;
+       opt->max_depth = def->max_depth;
+       opt->pathname = def->pathname;
+       opt->regflags = def->regflags;
+       opt->relative = def->relative;
+
+       strcpy(opt->color_context, def->color_context);
+       strcpy(opt->color_filename, def->color_filename);
+       strcpy(opt->color_function, def->color_function);
+       strcpy(opt->color_lineno, def->color_lineno);
+       strcpy(opt->color_match, def->color_match);
+       strcpy(opt->color_selected, def->color_selected);
+       strcpy(opt->color_sep, def->color_sep);
+}
+
+void grep_commit_pattern_type(enum grep_pattern_type pattern_type, struct grep_opt *opt)
+{
+       if (pattern_type != GREP_PATTERN_TYPE_UNSPECIFIED)
+               grep_set_pattern_type_option(pattern_type, opt);
+       else if (opt->pattern_type_option != GREP_PATTERN_TYPE_UNSPECIFIED)
+               grep_set_pattern_type_option(opt->pattern_type_option, opt);
+       else if (opt->extended_regexp_option)
+               grep_set_pattern_type_option(GREP_PATTERN_TYPE_ERE, opt);
+}
+
+void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct grep_opt *opt)
+{
+       switch (pattern_type) {
+       case GREP_PATTERN_TYPE_UNSPECIFIED:
+               /* fall through */
+
+       case GREP_PATTERN_TYPE_BRE:
+               opt->fixed = 0;
+               opt->pcre = 0;
+               opt->regflags &= ~REG_EXTENDED;
+               break;
+
+       case GREP_PATTERN_TYPE_ERE:
+               opt->fixed = 0;
+               opt->pcre = 0;
+               opt->regflags |= REG_EXTENDED;
+               break;
+
+       case GREP_PATTERN_TYPE_FIXED:
+               opt->fixed = 1;
+               opt->pcre = 0;
+               opt->regflags &= ~REG_EXTENDED;
+               break;
+
+       case GREP_PATTERN_TYPE_PCRE:
+               opt->fixed = 0;
+               opt->pcre = 1;
+               opt->regflags &= ~REG_EXTENDED;
+               break;
+       }
+}
 
 static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,
                                        const char *origin, int no,

diff --git a/grep.h b/grep.h
index c256ac6e15b22e153623ae3395fb6ffc732b20b9..701c78459f95cb94487d99af1fcd0cbfe738d617 100644 (file)
--- a/grep.h
+++ b/grep.h
@@ -138,6 +138,12 @@ struct grep_opt {
        void *output_priv;
 };
 
+extern void init_grep_defaults(void);
+extern int grep_config(const char *var, const char *value, void *);
+extern void grep_init(struct grep_opt *, const char *prefix);
+void grep_set_pattern_type_option(enum grep_pattern_type, struct grep_opt *opt);
+void grep_commit_pattern_type(enum grep_pattern_type, struct grep_opt *opt);
+
 extern void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen, const char *origin, int no, enum grep_pat_token t);
 extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *origin, int no, enum grep_pat_token t);
 extern void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const char *);

diff --git a/revision.c b/revision.c
index a09e60bedbbbf2c6bdeeacc3d1032bcc1be9cb55..d7d621cdbf224f5a2bc1780e9602dcd3d08c1d57 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -1048,9 +1048,9 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 
        revs->commit_format = CMIT_FMT_DEFAULT;
 
+       init_grep_defaults();
+       grep_init(&revs->grep_filter, prefix);
        revs->grep_filter.status_only = 1;
-       revs->grep_filter.pattern_tail = &(revs->grep_filter.pattern_list);
-       revs->grep_filter.header_tail = &(revs->grep_filter.header_list);
        revs->grep_filter.regflags = REG_NEWLINE;
 
        diff_setup(&revs->diffopt);
@@ -1603,13 +1603,17 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
                return argcount;
        } else if (!strcmp(arg, "--grep-debug")) {
                revs->grep_filter.debug = 1;
+       } else if (!strcmp(arg, "--basic-regexp")) {
+               grep_set_pattern_type_option(GREP_PATTERN_TYPE_BRE, &revs->grep_filter);
        } else if (!strcmp(arg, "--extended-regexp") || !strcmp(arg, "-E")) {
-               revs->grep_filter.regflags |= REG_EXTENDED;
+               grep_set_pattern_type_option(GREP_PATTERN_TYPE_ERE, &revs->grep_filter);
        } else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
                revs->grep_filter.regflags |= REG_ICASE;
                DIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);
        } else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
-               revs->grep_filter.fixed = 1;
+               grep_set_pattern_type_option(GREP_PATTERN_TYPE_FIXED, &revs->grep_filter);
+       } else if (!strcmp(arg, "--perl-regexp")) {
+               grep_set_pattern_type_option(GREP_PATTERN_TYPE_PCRE, &revs->grep_filter);
        } else if (!strcmp(arg, "--all-match")) {
                revs->grep_filter.all_match = 1;
        } else if ((argcount = parse_long_opt("encoding", argv, &optarg))) {
@@ -1893,6 +1897,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
        revs->diffopt.abbrev = revs->abbrev;
        diff_setup_done(&revs->diffopt);
 
+       grep_commit_pattern_type(GREP_PATTERN_TYPE_UNSPECIFIED,
+                                &revs->grep_filter);
        compile_grep_patterns(&revs->grep_filter);
 
        if (revs->reverse && revs->reflog_info)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 924ba536ca9718da0f4126d53c316a5f7a1926a5..e6537abe1d611aa2d96215fb263f76c5c0cf3b1a 100755 (executable)
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -230,6 +230,12 @@ test_expect_success 'log --grep -i' '
        test_cmp expect actual
 '
 
+test_expect_success 'log -F -E --grep=<ere> uses ere' '
+       echo second >expect &&
+       git log -1 --pretty="tformat:%s" -F -E --grep=s.c.nd >actual &&
+       test_cmp expect actual
+'
+
 cat > expect <<EOF
 * Second
 * sixth