grep: further simplify setting the pattern type

When c5c31d33 (grep: move pattern-type bits support to top-level grep.[ch], 2012-10-03) introduced grep_commit_pattern_type() helper function, the intention was to allow the users of grep API to having to fiddle only with .pattern_type_option (which can be set to "fixed", "basic", "extended", and "pcre"), and then immediately before compiling the pattern strings for use, call grep_commit_pattern_type() to have it prepare various bits in the grep_opt structure (like .fixed, .regflags, etc.). However, grep_set_pattern_type_option() helper function the grep API internally uses were left as an external function by mistake. This function shouldn't have been made callable by the users of the API. Later when the grep API was used in revision traversal machinery, the caller then mistakenly started calling the function around 34a4ae55 (log --grep: use the same helper to set -E/-F options as "git grep", 2012-10-03), instead of setting the .pattern_type_option field and letting the grep_commit_pattern_type() to take care of the details. This caused an unnecessary bug that made a configured grep.patternType take precedence over the command line options (e.g. --basic-regexp, --fixed-strings) in "git log" family of commands. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Jul 22, 2016 at 11:43 UTC 8465541e8ce8eaf16e66ab847086779768c18f2d
4 files changed +29 -16
grep.c
+11 -11
@@ -161,17 +161,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)
161 strcpy(opt->color_sep, def->color_sep);
162 }
163
164 -void grep_commit_pattern_type(enum grep_pattern_type pattern_type, struct grep_opt *opt)
165 -{
166 - if (pattern_type != GREP_PATTERN_TYPE_UNSPECIFIED)
167 - grep_set_pattern_type_option(pattern_type, opt);
168 - else if (opt->pattern_type_option != GREP_PATTERN_TYPE_UNSPECIFIED)
169 - grep_set_pattern_type_option(opt->pattern_type_option, opt);
170 - else if (opt->extended_regexp_option)
171 - grep_set_pattern_type_option(GREP_PATTERN_TYPE_ERE, opt);
172 -}
173 -
174 -void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct grep_opt *opt)
164 +static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct grep_opt *opt)
165 {
166 switch (pattern_type) {
167 case GREP_PATTERN_TYPE_UNSPECIFIED:
@@ -203,6 +193,16 @@ void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct gr
193 }
194 }
195
196 +void grep_commit_pattern_type(enum grep_pattern_type pattern_type, struct grep_opt *opt)
197 +{
198 + if (pattern_type != GREP_PATTERN_TYPE_UNSPECIFIED)
199 + grep_set_pattern_type_option(pattern_type, opt);
200 + else if (opt->pattern_type_option != GREP_PATTERN_TYPE_UNSPECIFIED)
201 + grep_set_pattern_type_option(opt->pattern_type_option, opt);
202 + else if (opt->extended_regexp_option)
203 + grep_set_pattern_type_option(GREP_PATTERN_TYPE_ERE, opt);
204 +}
205 +
206 static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,
207 const char *origin, int no,
208 enum grep_pat_token t,
grep.h
-1
@@ -144,7 +144,6 @@ struct grep_opt {
144 extern void init_grep_defaults(void);
145 extern int grep_config(const char *var, const char *value, void *);
146 extern void grep_init(struct grep_opt *, const char *prefix);
147 -void grep_set_pattern_type_option(enum grep_pattern_type, struct grep_opt *opt);
147 void grep_commit_pattern_type(enum grep_pattern_type, struct grep_opt *opt);
148
149 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);
revision.c
+4 -4
@@ -1964,16 +1964,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
1964 } else if (!strcmp(arg, "--grep-debug")) {
1965 revs->grep_filter.debug = 1;
1966 } else if (!strcmp(arg, "--basic-regexp")) {
1967 - grep_set_pattern_type_option(GREP_PATTERN_TYPE_BRE, &revs->grep_filter);
1967 + revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_BRE;
1968 } else if (!strcmp(arg, "--extended-regexp") || !strcmp(arg, "-E")) {
1969 - grep_set_pattern_type_option(GREP_PATTERN_TYPE_ERE, &revs->grep_filter);
1969 + revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_ERE;
1970 } else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
1971 revs->grep_filter.regflags |= REG_ICASE;
1972 DIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);
1973 } else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
1974 - grep_set_pattern_type_option(GREP_PATTERN_TYPE_FIXED, &revs->grep_filter);
1974 + revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_FIXED;
1975 } else if (!strcmp(arg, "--perl-regexp")) {
1976 - grep_set_pattern_type_option(GREP_PATTERN_TYPE_PCRE, &revs->grep_filter);
1976 + revs->grep_filter.pattern_type_option = GREP_PATTERN_TYPE_PCRE;
1977 } else if (!strcmp(arg, "--all-match")) {
1978 revs->grep_filter.all_match = 1;
1979 } else if (!strcmp(arg, "--invert-grep")) {
t/t4202-log.sh
+14
@@ -232,6 +232,20 @@ test_expect_success 'log -F -E --grep=<ere> uses ere' '
232 test_cmp expect actual
233 '
234
235 +test_expect_success 'log with grep.patternType configuration' '
236 + >expect &&
237 + git -c grep.patterntype=fixed \
238 + log -1 --pretty=tformat:%s --grep=s.c.nd >actual &&
239 + test_cmp expect actual
240 +'
241 +
242 +test_expect_success 'log with grep.patternType configuration and command line' '
243 + echo second >expect &&
244 + git -c grep.patterntype=fixed \
245 + log -1 --pretty=tformat:%s --basic-regexp --grep=s.c.nd >actual &&
246 + test_cmp expect actual
247 +'
248 +
249 cat > expect <<EOF
250 * Second
251 * sixth