Skip to content

Commit 5b84280

Browse files
gitstergitster
committed
Merge branch 'ab/grep-patterntype'
Some code clean-up in the "git grep" machinery. * ab/grep-patterntype: grep: simplify config parsing and option parsing grep.c: do "if (bool && memchr())" not "if (memchr() && bool)" grep.h: make "grep_opt.pattern_type_option" use its enum grep API: call grep_config() after grep_init() grep.c: don't pass along NULL callback value built-ins: trust the "prefix" from run_builtin() grep tests: add missing "grep.patternType" config tests grep tests: create a helper function for "BRE" or "ERE" log tests: check if grep_config() is called by "log"-like cmds grep.h: remove unused "regex_t regexp" from grep_opt
2 parents 2e65591 + 04bf052 commit 5b84280

File tree

9 files changed

+195
-206
lines changed

9 files changed

+195
-206
lines changed

‎builtin/grep.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
#include "object-store.h"
2727
#include "packfile.h"
2828

29+
static const char *grep_prefix;
30+
2931
static char const * const grep_usage[] = {
3032
N_("git grep [<options>] [-e] <pattern> [<rev>...] [[--] <path>...]"),
3133
NULL
@@ -284,7 +286,7 @@ static int wait_all(void)
284286
static int grep_cmd_config(const char *var, const char *value, void *cb)
285287
{
286288
int st = grep_config(var, value, cb);
287-
if (git_color_default_config(var, value, cb) < 0)
289+
if (git_color_default_config(var, value, NULL) < 0)
288290
st = -1;
289291

290292
if (!strcmp(var, "grep.threads")) {
@@ -315,11 +317,11 @@ static void grep_source_name(struct grep_opt *opt, const char *filename,
315317
strbuf_reset(out);
316318

317319
if (opt->null_following_name) {
318-
if (opt->relative && opt->prefix_length) {
320+
if (opt->relative && grep_prefix) {
319321
struct strbuf rel_buf = STRBUF_INIT;
320322
const char *rel_name =
321323
relative_path(filename + tree_name_len,
322-
opt->prefix, &rel_buf);
324+
grep_prefix, &rel_buf);
323325

324326
if (tree_name_len)
325327
strbuf_add(out, filename, tree_name_len);
@@ -332,8 +334,8 @@ static void grep_source_name(struct grep_opt *opt, const char *filename,
332334
return;
333335
}
334336

335-
if (opt->relative && opt->prefix_length)
336-
quote_path(filename + tree_name_len, opt->prefix, out, 0);
337+
if (opt->relative && grep_prefix)
338+
quote_path(filename + tree_name_len, grep_prefix, out, 0);
337339
else
338340
quote_c_style(filename + tree_name_len, out, NULL, 0);
339341

@@ -843,7 +845,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
843845
int i;
844846
int dummy;
845847
int use_index = 1;
846-
int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
847848
int allow_revs;
848849

849850
struct option options[] = {
@@ -877,16 +878,16 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
877878
N_("descend at most <depth> levels"), PARSE_OPT_NONEG,
878879
NULL, 1 },
879880
OPT_GROUP(""),
880-
OPT_SET_INT('E', "extended-regexp", &pattern_type_arg,
881+
OPT_SET_INT('E', "extended-regexp", &opt.pattern_type_option,
881882
N_("use extended POSIX regular expressions"),
882883
GREP_PATTERN_TYPE_ERE),
883-
OPT_SET_INT('G', "basic-regexp", &pattern_type_arg,
884+
OPT_SET_INT('G', "basic-regexp", &opt.pattern_type_option,
884885
N_("use basic POSIX regular expressions (default)"),
885886
GREP_PATTERN_TYPE_BRE),
886-
OPT_SET_INT('F', "fixed-strings", &pattern_type_arg,
887+
OPT_SET_INT('F', "fixed-strings", &opt.pattern_type_option,
887888
N_("interpret patterns as fixed strings"),
888889
GREP_PATTERN_TYPE_FIXED),
889-
OPT_SET_INT('P', "perl-regexp", &pattern_type_arg,
890+
OPT_SET_INT('P', "perl-regexp", &opt.pattern_type_option,
890891
N_("use Perl-compatible regular expressions"),
891892
GREP_PATTERN_TYPE_PCRE),
892893
OPT_GROUP(""),
@@ -962,9 +963,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
962963
PARSE_OPT_NOCOMPLETE),
963964
OPT_END()
964965
};
966+
grep_prefix = prefix;
965967

966-
git_config(grep_cmd_config, NULL);
967-
grep_init(&opt, the_repository, prefix);
968+
grep_init(&opt, the_repository);
969+
git_config(grep_cmd_config, &opt);
968970

969971
/*
970972
* If there is no -- then the paths must exist in the working
@@ -979,7 +981,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
979981
argc = parse_options(argc, argv, prefix, options, grep_usage,
980982
PARSE_OPT_KEEP_DASHDASH |
981983
PARSE_OPT_STOP_AT_NON_OPTION);
982-
grep_commit_pattern_type(pattern_type_arg, &opt);
983984

984985
if (use_index && !startup_info->have_repository) {
985986
int fallback = 0;

‎builtin/log.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -533,8 +533,6 @@ static int git_log_config(const char *var, const char *value, void *cb)
533533
return 0;
534534
}
535535

536-
if (grep_config(var, value, cb) < 0)
537-
return -1;
538536
if (git_gpg_config(var, value, cb) < 0)
539537
return -1;
540538
return git_diff_ui_config(var, value, cb);
@@ -549,6 +547,8 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
549547
git_config(git_log_config, NULL);
550548

551549
repo_init_revisions(the_repository, &rev, prefix);
550+
git_config(grep_config, &rev.grep_filter);
551+
552552
rev.diff = 1;
553553
rev.simplify_history = 0;
554554
memset(&opt, 0, sizeof(opt));
@@ -663,6 +663,8 @@ int cmd_show(int argc, const char **argv, const char *prefix)
663663

664664
memset(&match_all, 0, sizeof(match_all));
665665
repo_init_revisions(the_repository, &rev, prefix);
666+
git_config(grep_config, &rev.grep_filter);
667+
666668
rev.diff = 1;
667669
rev.always_show_header = 1;
668670
rev.no_walk = 1;
@@ -746,6 +748,8 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
746748

747749
repo_init_revisions(the_repository, &rev, prefix);
748750
init_reflog_walk(&rev.reflog_info);
751+
git_config(grep_config, &rev.grep_filter);
752+
749753
rev.verbose_header = 1;
750754
memset(&opt, 0, sizeof(opt));
751755
opt.def = "HEAD";
@@ -779,6 +783,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)
779783
git_config(git_log_config, NULL);
780784

781785
repo_init_revisions(the_repository, &rev, prefix);
786+
git_config(grep_config, &rev.grep_filter);
787+
782788
rev.always_show_header = 1;
783789
memset(&opt, 0, sizeof(opt));
784790
opt.def = "HEAD";
@@ -1861,10 +1867,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
18611867
extra_hdr.strdup_strings = 1;
18621868
extra_to.strdup_strings = 1;
18631869
extra_cc.strdup_strings = 1;
1870+
18641871
init_log_defaults();
18651872
init_display_notes(&notes_opt);
18661873
git_config(git_format_config, NULL);
18671874
repo_init_revisions(the_repository, &rev, prefix);
1875+
git_config(grep_config, &rev.grep_filter);
1876+
18681877
rev.show_notes = show_notes;
18691878
memcpy(&rev.notes_opt, &notes_opt, sizeof(notes_opt));
18701879
rev.commit_format = CMIT_FMT_EMAIL;

‎builtin/ls-tree.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
150150

151151
git_config(git_default_config, NULL);
152152
ls_tree_prefix = prefix;
153-
if (prefix && *prefix)
153+
if (prefix)
154154
chomp_prefix = strlen(prefix);
155155

156156
argc = parse_options(argc, argv, prefix, ls_tree_options,

‎git.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
436436
} else {
437437
prefix = NULL;
438438
}
439+
assert(!prefix || *prefix);
439440
precompose_argv_prefix(argc, argv, NULL);
440441
if (use_pager == -1 && run_setup &&
441442
!(p->option & DELAY_PAGER_CONFIG))

‎grep.c

Lines changed: 14 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -19,27 +19,6 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
1919
fwrite(buf, size, 1, stdout);
2020
}
2121

22-
static struct grep_opt grep_defaults = {
23-
.relative = 1,
24-
.pathname = 1,
25-
.max_depth = -1,
26-
.pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED,
27-
.colors = {
28-
[GREP_COLOR_CONTEXT] = "",
29-
[GREP_COLOR_FILENAME] = GIT_COLOR_MAGENTA,
30-
[GREP_COLOR_FUNCTION] = "",
31-
[GREP_COLOR_LINENO] = GIT_COLOR_GREEN,
32-
[GREP_COLOR_COLUMNNO] = GIT_COLOR_GREEN,
33-
[GREP_COLOR_MATCH_CONTEXT] = GIT_COLOR_BOLD_RED,
34-
[GREP_COLOR_MATCH_SELECTED] = GIT_COLOR_BOLD_RED,
35-
[GREP_COLOR_SELECTED] = "",
36-
[GREP_COLOR_SEP] = GIT_COLOR_CYAN,
37-
},
38-
.only_matching = 0,
39-
.color = -1,
40-
.output = std_output,
41-
};
42-
4322
static const char *color_grep_slots[] = {
4423
[GREP_COLOR_CONTEXT] = "context",
4524
[GREP_COLOR_FILENAME] = "filename",
@@ -75,20 +54,12 @@ define_list_config_array_extra(color_grep_slots, {"match"});
7554
*/
7655
int grep_config(const char *var, const char *value, void *cb)
7756
{
78-
struct grep_opt *opt = &grep_defaults;
57+
struct grep_opt *opt = cb;
7958
const char *slot;
8059

8160
if (userdiff_config(var, value) < 0)
8261
return -1;
8362

84-
/*
85-
* The instance of grep_opt that we set up here is copied by
86-
* grep_init() to be used by each individual invocation.
87-
* When populating a new field of this structure here, be
88-
* sure to think about ownership -- e.g., you might need to
89-
* override the shallow copy in grep_init() with a deep copy.
90-
*/
91-
9263
if (!strcmp(var, "grep.extendedregexp")) {
9364
opt->extended_regexp_option = git_config_bool(var, value);
9465
return 0;
@@ -134,78 +105,16 @@ int grep_config(const char *var, const char *value, void *cb)
134105
return 0;
135106
}
136107

137-
/*
138-
* Initialize one instance of grep_opt and copy the
139-
* default values from the template we read the configuration
140-
* information in an earlier call to git_config(grep_config).
141-
*/
142-
void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix)
108+
void grep_init(struct grep_opt *opt, struct repository *repo)
143109
{
144-
*opt = grep_defaults;
110+
struct grep_opt blank = GREP_OPT_INIT;
111+
memcpy(opt, &blank, sizeof(*opt));
145112

146113
opt->repo = repo;
147-
opt->prefix = prefix;
148-
opt->prefix_length = (prefix && *prefix) ? strlen(prefix) : 0;
149114
opt->pattern_tail = &opt->pattern_list;
150115
opt->header_tail = &opt->header_list;
151116
}
152117

153-
static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct grep_opt *opt)
154-
{
155-
/*
156-
* When committing to the pattern type by setting the relevant
157-
* fields in grep_opt it's generally not necessary to zero out
158-
* the fields we're not choosing, since they won't have been
159-
* set by anything. The extended_regexp_option field is the
160-
* only exception to this.
161-
*
162-
* This is because in the process of parsing grep.patternType
163-
* & grep.extendedRegexp we set opt->pattern_type_option and
164-
* opt->extended_regexp_option, respectively. We then
165-
* internally use opt->extended_regexp_option to see if we're
166-
* compiling an ERE. It must be unset if that's not actually
167-
* the case.
168-
*/
169-
if (pattern_type != GREP_PATTERN_TYPE_ERE &&
170-
opt->extended_regexp_option)
171-
opt->extended_regexp_option = 0;
172-
173-
switch (pattern_type) {
174-
case GREP_PATTERN_TYPE_UNSPECIFIED:
175-
/* fall through */
176-
177-
case GREP_PATTERN_TYPE_BRE:
178-
break;
179-
180-
case GREP_PATTERN_TYPE_ERE:
181-
opt->extended_regexp_option = 1;
182-
break;
183-
184-
case GREP_PATTERN_TYPE_FIXED:
185-
opt->fixed = 1;
186-
break;
187-
188-
case GREP_PATTERN_TYPE_PCRE:
189-
opt->pcre2 = 1;
190-
break;
191-
}
192-
}
193-
194-
void grep_commit_pattern_type(enum grep_pattern_type pattern_type, struct grep_opt *opt)
195-
{
196-
if (pattern_type != GREP_PATTERN_TYPE_UNSPECIFIED)
197-
grep_set_pattern_type_option(pattern_type, opt);
198-
else if (opt->pattern_type_option != GREP_PATTERN_TYPE_UNSPECIFIED)
199-
grep_set_pattern_type_option(opt->pattern_type_option, opt);
200-
else if (opt->extended_regexp_option)
201-
/*
202-
* This branch *must* happen after setting from the
203-
* opt->pattern_type_option above, we don't want
204-
* grep.extendedRegexp to override grep.patternType!
205-
*/
206-
grep_set_pattern_type_option(GREP_PATTERN_TYPE_ERE, opt);
207-
}
208-
209118
static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,
210119
const char *origin, int no,
211120
enum grep_pat_token t,
@@ -523,11 +432,17 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
523432
int err;
524433
int regflags = REG_NEWLINE;
525434

435+
if (opt->pattern_type_option == GREP_PATTERN_TYPE_UNSPECIFIED)
436+
opt->pattern_type_option = (opt->extended_regexp_option
437+
? GREP_PATTERN_TYPE_ERE
438+
: GREP_PATTERN_TYPE_BRE);
439+
526440
p->word_regexp = opt->word_regexp;
527441
p->ignore_case = opt->ignore_case;
528-
p->fixed = opt->fixed;
442+
p->fixed = opt->pattern_type_option == GREP_PATTERN_TYPE_FIXED;
529443

530-
if (memchr(p->pattern, 0, p->patternlen) && !opt->pcre2)
444+
if (opt->pattern_type_option != GREP_PATTERN_TYPE_PCRE &&
445+
memchr(p->pattern, 0, p->patternlen))
531446
die(_("given pattern contains NULL byte (via -f <file>). This is only supported with -P under PCRE v2"));
532447

533448
p->is_fixed = is_fixed(p->pattern, p->patternlen);
@@ -578,14 +493,14 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
578493
return;
579494
}
580495

581-
if (opt->pcre2) {
496+
if (opt->pattern_type_option == GREP_PATTERN_TYPE_PCRE) {
582497
compile_pcre2_pattern(p, opt);
583498
return;
584499
}
585500

586501
if (p->ignore_case)
587502
regflags |= REG_ICASE;
588-
if (opt->extended_regexp_option)
503+
if (opt->pattern_type_option == GREP_PATTERN_TYPE_ERE)
589504
regflags |= REG_EXTENDED;
590505
err = regcomp(&p->regexp, p->pattern, regflags);
591506
if (err) {

0 commit comments

Comments
 (0)