threads / patch / 31777

v2, 7 partsTying loose ends on grep-pcre

Subject: [PATCH v2 0/7] Tying loose ends on grep-pcre

## tl;dr

11 messages between Oct 10, 2012 and Oct 11, 2012. Diffs are folded; open one to read it.

replies: 10people: 2as markdown or json

Junio C Hamano· Oct 10, 2012, 07:55 UTC · lore

It took longer than expected, but here is a reroll of the previous series to bring more recent "git grep" enhancements to the "--grep" option of commands in "git log" family.

The early part of the series (1-3) refactors the code that reads configuration items related to "grep" and the code that mixes the result with the command line options to prepare grep_opt, which so far lived in builtin/grep.c, and moves them to the grep.[ch] at the top-level.

The middle part (4-6) reuses the code to set-up grep_opt refactored by the earlier part of the series on revs->grep_filter that is used in "git log --grep=..." processing. It incidentally fixes a small bug where "git log -F -E --grep='<ere>'" did not look for matches to the pattern in extended regular expression, and adds --basic-regexp and --perl-regexp command line options to "git log" family for completeness.

The last one teaches "git log" family to honor the "grep.*" configuration variables, e.g. "grep.patterntype", so that you can say "git -c grep.patterntype=perl log --grep='(?:pcre)'".

Obviously, it is too late for this cycle and will not graduate to 'master' before the 1.8.0 final.

Junio C Hamano (7):
  builtin/grep.c: make configuration callback more reusable
  grep: move the configuration parsing logic to grep.[ch]
  grep: move pattern-type bits support to top-level grep.[ch]
  revisions: initialize revs->grep_filter using grep_init()
  log --grep: use the same helper to set -E/-F options as "git grep"
  log --grep: accept --basic-regexp and --perl-regexp
  log: honor grep.* configuration
 Documentation/rev-list-options.txt |  10 +++
 builtin/grep.c                     | 133 ++--------------------------
 builtin/log.c                      |   8 +-
 grep.c                             | 177 +++++++++++++++++++++++++++++++++++++
 grep.h                             |   6 ++
 revision.c                         |  14 ++-
 t/t4202-log.sh                     |   6 ++
 7 files changed, 225 insertions(+), 129 deletions(-)
-- 
1.8.0.rc1.76.g5a375e6
Junio C Hamano· Oct 10, 2012, 07:55 UTC · re: Junio C Hamano · lore

[PATCH v2 1/7] builtin/grep.c: make configuration callback more reusable

The grep_config() function takes one instance of grep_opt as its callback parameter, and populates it by running git_config().

This has three practical implications:
 - You have to have an instance of grep_opt already when you call
   the configuration, but that is not necessarily always true.  You
   may be trying to initialize the grep_filter member of rev_info,
   but are not ready to call init_revisions() on it yet.
 - It is not easy to enhance grep_config() in such a way to make it
   cascade to other callback functions to grab other variables in
   one call of git_config(); grep_config() can be cascaded into from
   other callbacks, but it has to be at the leaf level of a cascade.
 - If you ever need to use more than one instance of grep_opt, you
   will have to open and read the configuration file(s) every time
   you initialize them.

Rearrange the configuration mechanism and model it after how diff configuration variables are handled. An early call to git_config() reads and remembers the values taken from the configuration in the default "template", and a separate call to grep_init() uses this template to instantiate a grep_opt.

The next step will be to move some of this out of this file so that the other user of the grep machinery (i.e. "log") can use it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/grep.c | 104 +++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 79 insertions(+), 25 deletions(-)
Show changes to builtin/grep.c +79 −25
diff --git a/builtin/grep.c b/builtin/grep.c
index 82530a6..83232c9 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -308,9 +308,41 @@ static void grep_pattern_type_options(const int pattern_type, struct grep_opt *o
 	}
 }
 
+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...
+ */
+static void init_grep_defaults(void)
+{
+	struct grep_opt *opt = &grep_defaults;
+
+	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;
+}
+
+/*
+ * Read the configuration file once and store it in
+ * the grep_defaults template.
+ */
 static int grep_config(const char *var, const char *value, void *cb)
 {
-	struct grep_opt *opt = cb;
+	struct grep_opt *opt = &grep_defaults;
 	char *color = NULL;
 
 	if (userdiff_config(var, value) < 0)
@@ -327,7 +359,7 @@ static int grep_config(const char *var, const char *value, void *cb)
 	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);
@@ -350,8 +382,7 @@ static int grep_config(const char *var, const char *value, void *cb)
 		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);
@@ -360,6 +391,47 @@ static int grep_config(const char *var, const char *value, void *cb)
 	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).
+ */
+static 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);
+}
+
+static int grep_cmd_config(const char *var, const char *value, void *cb)
+{
+	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)
 {
 	void *data;
@@ -839,27 +911,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

-- 
1.8.0.rc1.76.g5a375e6
Junio C Hamano· Oct 10, 2012, 07:55 UTC · re: Junio C Hamano · lore

[PATCH v2 2/7] grep: move the configuration parsing logic to grep.[ch]

The configuration handling is a library-ish part of this program, that is not specific to "git grep" command. It should be reusable by "log" and others.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/grep.c | 131 ---------------------------------------------------------
 grep.c         | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 grep.h         |   4 ++
 3 files changed, 134 insertions(+), 131 deletions(-)
Show changes to 3 files +134 −131

builtin/grep.c, grep.c, grep.h

diff --git a/builtin/grep.c b/builtin/grep.c
index 83232c9..b63a9f8 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -261,21 +261,6 @@ static int wait_all(void)
 }
 #endif
 
-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);
-}
-
 static void grep_pattern_type_options(const int pattern_type, struct grep_opt *opt)
 {
 	switch (pattern_type) {
@@ -308,122 +293,6 @@ static void grep_pattern_type_options(const int pattern_type, struct grep_opt *o
 	}
 }
 
-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...
- */
-static void init_grep_defaults(void)
-{
-	struct grep_opt *opt = &grep_defaults;
-
-	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;
-}
-
-/*
- * Read the configuration file once and store it in
- * the grep_defaults template.
- */
-static 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).
- */
-static 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);
-}
-
 static int grep_cmd_config(const char *var, const char *value, void *cb)
 {
 	int st = grep_config(var, value, cb);
diff --git a/grep.c b/grep.c
index edc7776..621e6ec 100644
--- a/grep.c
+++ b/grep.c
@@ -6,6 +6,136 @@
 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;
+
+	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);
+}
 
 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 c256ac6..9aa1cc7 100644
--- a/grep.h
+++ b/grep.h
@@ -138,6 +138,10 @@ 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);
+
 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 *);

-- 
1.8.0.rc1.76.g5a375e6
Junio C Hamano· Oct 10, 2012, 07:55 UTC · re: Junio C Hamano · lore

[PATCH v2 3/7] grep: move pattern-type bits support to top-level grep.[ch]

Switching between -E/-G/-P/-F correctly needs a lot more than just flipping opt->regflags bit these days, and we have a nice helper function buried in builtin/grep.c for the sole use of "git grep".

Extract it so that "log --grep" family can also use it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/grep.c | 40 +---------------------------------------
 grep.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
 grep.h         |  2 ++
 3 files changed, 45 insertions(+), 39 deletions(-)
Show changes to 3 files +45 −39

builtin/grep.c, grep.c, grep.h

diff --git a/builtin/grep.c b/builtin/grep.c
index b63a9f8..c296e6f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -261,38 +261,6 @@ static int wait_all(void)
 }
 #endif
 
-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_cmd_config(const char *var, const char *value, void *cb)
 {
 	int st = grep_config(var, value, cb);
@@ -798,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/grep.c b/grep.c
index 621e6ec..279a559 100644
--- a/grep.c
+++ b/grep.c
@@ -137,6 +137,48 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	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,
 					enum grep_pat_token t,
diff --git a/grep.h b/grep.h
index 9aa1cc7..701c784 100644
--- a/grep.h
+++ b/grep.h
@@ -141,6 +141,8 @@ struct grep_opt {
 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);

-- 
1.8.0.rc1.76.g5a375e6
Junio C Hamano· Oct 10, 2012, 07:55 UTC · re: Junio C Hamano · lore

[PATCH v2 4/7] revisions: initialize revs->grep_filter using grep_init()

Instead of using the hand-rolled initialization sequence, use grep_init() to populate the necessary bits. This opens the door to allow the calling commands to optionally read grep.* configuration variables via git_config() if they want to.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 grep.c     | 5 +++++
 revision.c | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)
Show changes to 2 files +9 −2

grep.c, revision.c

diff --git a/grep.c b/grep.c
index 279a559..a947a68 100644
--- a/grep.c
+++ b/grep.c
@@ -16,6 +16,11 @@ static struct grep_opt grep_defaults;
 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;
diff --git a/revision.c b/revision.c
index a09e60b..a4a9314 100644
--- 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);
@@ -1893,6 +1893,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)

-- 
1.8.0.rc1.76.g5a375e6
Junio C Hamano· Oct 10, 2012, 07:55 UTC · re: Junio C Hamano · lore

[PATCH v2 5/7] log --grep: use the same helper to set -E/-F options as "git grep"

The command line option parser for "git log -F -E --grep='<ere>'" did not flip the "fixed" bit, violating the general "last option wins" principle among conflicting options.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 revision.c     | 4 ++--
 t/t4202-log.sh | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)
Show changes to 2 files +8 −2

revision.c, t/t4202-log.sh

diff --git a/revision.c b/revision.c
index a4a9314..68545c8 100644
--- a/revision.c
+++ b/revision.c
@@ -1604,12 +1604,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--grep-debug")) {
 		revs->grep_filter.debug = 1;
 	} 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, "--all-match")) {
 		revs->grep_filter.all_match = 1;
 	} else if ((argcount = parse_long_opt("encoding", argv, &optarg))) {
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 924ba53..e6537ab 100755
--- 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

-- 
1.8.0.rc1.76.g5a375e6
Junio C Hamano· Oct 10, 2012, 07:55 UTC · re: Junio C Hamano · lore

[PATCH v2 6/7] log --grep: accept --basic-regexp and --perl-regexp

When we added the "--perl-regexp" option (or "-P") to "git grep", we should have done the same for the commands in the "git log" family, but somehow we forgot to do so. This corrects it, but we will reserve the short-and-sweet "-P" option for something else for now.

Also introduce the "--basic-regexp" option for completeness, so that the "last one wins" principle can be used to defeat an earlier -E option, e.g. "git log -E --basic-regexp --grep='<bre>'". Note that it cannot have the short "-G" option as the option is to grep in the patch text in the context of "log" family.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/rev-list-options.txt | 10 ++++++++++
 revision.c                         |  4 ++++
 2 files changed, 14 insertions(+)
Show changes to 2 files +14 −0

Documentation/rev-list-options.txt, revision.c

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index ee49743..1ec14a0 100644
--- 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/revision.c b/revision.c
index 68545c8..d7d621c 100644
--- a/revision.c
+++ b/revision.c
@@ -1603,6 +1603,8 @@ 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")) {
 		grep_set_pattern_type_option(GREP_PATTERN_TYPE_ERE, &revs->grep_filter);
 	} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
@@ -1610,6 +1612,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		DIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);
 	} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
 		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))) {

-- 
1.8.0.rc1.76.g5a375e6
Junio C Hamano· Oct 10, 2012, 07:55 UTC · re: Junio C Hamano · lore

[PATCH v2 7/7] log: honor grep.* configuration

Now the grep_config() callback is reusable from other configuration callbacks, call it from git_log_config() so that grep.patterntype and friends can be used with the commands in the "git log" family.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/log.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
Show changes to builtin/log.c +7 −1
diff --git a/builtin/log.c b/builtin/log.c
index 09cf43e..e7b7db1 100644
--- 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;

-- 
1.8.0.rc1.76.g5a375e6
Michael Haggerty· Oct 10, 2012, 15:17 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 0/7] Tying loose ends on grep-pcre

On 10/10/2012 09:55 AM, Junio C Hamano wrote:
Show 21 quoted lines
> It took longer than expected, but here is a reroll of the previous
> series to bring more recent "git grep" enhancements to the "--grep"
> option of commands in "git log" family.
> 
> The early part of the series (1-3) refactors the code that reads
> configuration items related to "grep" and the code that mixes the
> result with the command line options to prepare grep_opt, which so
> far lived in builtin/grep.c, and moves them to the grep.[ch] at the
> top-level.
> 
> The middle part (4-6) reuses the code to set-up grep_opt refactored
> by the earlier part of the series on revs->grep_filter that is used
> in "git log --grep=..." processing.  It incidentally fixes a small
> bug where "git log -F -E --grep='<ere>'" did not look for matches to
> the pattern in extended regular expression, and adds --basic-regexp
> and --perl-regexp command line options to "git log" family for
> completeness.
> 
> The last one teaches "git log" family to honor the "grep.*"
> configuration variables, e.g. "grep.patterntype", so that you can
> say "git -c grep.patterntype=perl log --grep='(?:pcre)'".

Maybe this has been discussed already, but it seems to me that adding a persistent setting that affects how "git log --grep" interprets the pattern argument could break some scripts that assume that the "old" interpretation is always used. Shouldn't this at least be documented as a backwards incompatibility?

Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Junio C Hamano· Oct 10, 2012, 16:52 UTC · re: Michael Haggerty · lore

Re: [PATCH v2 0/7] Tying loose ends on grep-pcre

Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 9 quoted lines
>> The last one teaches "git log" family to honor the "grep.*"
>> configuration variables, e.g. "grep.patterntype", so that you can
>> say "git -c grep.patterntype=perl log --grep='(?:pcre)'".
>
> Maybe this has been discussed already, but it seems to me that adding a
> persistent setting that affects how "git log --grep" interprets the
> pattern argument could break some scripts that assume that the "old"
> interpretation is always used.  Shouldn't this at least be documented as
> a backwards incompatibility?

If somebody scripts around "log" with hardcoded query "--grep=..." strings, they can force a particular variant from such a command line at the same time. But as always, responsibility of doing so is on the person who writes such a script; "log" being a Porcelain, we value ease-of-use in interactive case more than cast-in-stone interface stability like we do for plumbing commands.

And that is exactly why the series avoids changing the behaviour for the "rev-list" plumbing.

Michael Haggerty· Oct 11, 2012, 07:08 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 0/7] Tying loose ends on grep-pcre

On 10/10/2012 06:52 PM, Junio C Hamano wrote:
Show 21 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>>> The last one teaches "git log" family to honor the "grep.*"
>>> configuration variables, e.g. "grep.patterntype", so that you can
>>> say "git -c grep.patterntype=perl log --grep='(?:pcre)'".
>>
>> Maybe this has been discussed already, but it seems to me that adding a
>> persistent setting that affects how "git log --grep" interprets the
>> pattern argument could break some scripts that assume that the "old"
>> interpretation is always used.  Shouldn't this at least be documented as
>> a backwards incompatibility?
> 
> If somebody scripts around "log" with hardcoded query "--grep=..."
> strings, they can force a particular variant from such a command
> line at the same time.  But as always, responsibility of doing so is
> on the person who writes such a script; "log" being a Porcelain, we
> value ease-of-use in interactive case more than cast-in-stone
> interface stability like we do for plumbing commands.
> 
> And that is exactly why the series avoids changing the behaviour for
> the "rev-list" plumbing.
OK, you've convinced me.  Thanks.
Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

← back to recent threads