grep API: call grep_config() after grep_init()
The grep_init() function used the odd pattern of initializing the passed-in "struct grep_opt" with a statically defined "grep_defaults" struct, which would be modified in-place when we invoked grep_config(). So we effectively (b) initialized config, (a) then defaults, (c) followed by user options. Usually those are ordered as "a", "b" and "c" instead. As the comments being removed here show the previous behavior needed to be carefully explained as we'd potentially share the populated configuration among different instances of grep_init(). In practice we didn't do that, but now that it can't be a concern anymore let's remove those comments. This does not change the behavior of any of the configuration variables or options. That would have been the case if we didn't move around the grep_config() call in "builtin/log.c". But now that we call "grep_config" after "git_log_config" and "git_format_config" we'll need to pass in the already initialized "struct grep_opt *". See 6ba9bb76e02 (grep: copy struct in one fell swoop, 2020-11-29) and 7687a0541e0 (grep: move the configuration parsing logic to grep.[ch], 2012-10-09) for the commits that added the comments. The memcpy() pattern here will be optimized away and follows the convention of other *_init() functions. See 5726a6b4012 (*.c *_init(): define in terms of corresponding *_INIT macro, 2021-07-01). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ævar Arnfjörð Bjarmason committed
Feb 16, 2022 at 01:00 UTC
72365bb49923da065e7a43e61a912ef17f143c7f
4 files changed
+37
-40
builtin/grep.c
+2
-2
@@ -285,7 +285,7 @@ static int wait_all(void)
285
286
static int grep_cmd_config(const char *var, const char *value, void *cb)
287
{
288
- int st = grep_config(var, value, NULL);
288
+ int st = grep_config(var, value, cb);
289
if (git_color_default_config(var, value, NULL) < 0)
290
st = -1;
291
@@ -966,8 +966,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
966
};
967
grep_prefix = prefix;
968
969
- git_config(grep_cmd_config, NULL);
969
grep_init(&opt, the_repository);
970
+ git_config(grep_cmd_config, &opt);
971
972
/*
973
* If there is no -- then the paths must exist in the working
builtin/log.c
+11
-2
@@ -520,8 +520,6 @@ static int git_log_config(const char *var, const char *value, void *cb)
520
return 0;
521
}
522
523
- if (grep_config(var, value, cb) < 0)
524
- return -1;
523
if (git_gpg_config(var, value, cb) < 0)
524
return -1;
525
return git_diff_ui_config(var, value, cb);
@@ -536,6 +534,8 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
534
git_config(git_log_config, NULL);
535
536
repo_init_revisions(the_repository, &rev, prefix);
537
+ git_config(grep_config, &rev.grep_filter);
538
+
539
rev.diff = 1;
540
rev.simplify_history = 0;
541
memset(&opt, 0, sizeof(opt));
@@ -650,6 +650,8 @@ int cmd_show(int argc, const char **argv, const char *prefix)
650
651
memset(&match_all, 0, sizeof(match_all));
652
repo_init_revisions(the_repository, &rev, prefix);
653
+ git_config(grep_config, &rev.grep_filter);
654
+
655
rev.diff = 1;
656
rev.always_show_header = 1;
657
rev.no_walk = 1;
@@ -733,6 +735,8 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
735
736
repo_init_revisions(the_repository, &rev, prefix);
737
init_reflog_walk(&rev.reflog_info);
738
+ git_config(grep_config, &rev.grep_filter);
739
+
740
rev.verbose_header = 1;
741
memset(&opt, 0, sizeof(opt));
742
opt.def = "HEAD";
@@ -766,6 +770,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)
770
git_config(git_log_config, NULL);
771
772
repo_init_revisions(the_repository, &rev, prefix);
773
+ git_config(grep_config, &rev.grep_filter);
774
+
775
rev.always_show_header = 1;
776
memset(&opt, 0, sizeof(opt));
777
opt.def = "HEAD";
@@ -1848,10 +1854,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
1854
extra_hdr.strdup_strings = 1;
1855
extra_to.strdup_strings = 1;
1856
extra_cc.strdup_strings = 1;
1857
+
1858
init_log_defaults();
1859
init_display_notes(¬es_opt);
1860
git_config(git_format_config, NULL);
1861
repo_init_revisions(the_repository, &rev, prefix);
1862
+ git_config(grep_config, &rev.grep_filter);
1863
+
1864
rev.show_notes = show_notes;
1865
memcpy(&rev.notes_opt, ¬es_opt, sizeof(notes_opt));
1866
rev.commit_format = CMIT_FMT_EMAIL;
grep.c
+3
-36
@@ -19,27 +19,6 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
19
fwrite(buf, size, 1, stdout);
20
}
21
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
-
22
static const char *color_grep_slots[] = {
23
[GREP_COLOR_CONTEXT] = "context",
24
[GREP_COLOR_FILENAME] = "filename",
@@ -75,20 +54,12 @@ define_list_config_array_extra(color_grep_slots, {"match"});
54
*/
55
int grep_config(const char *var, const char *value, void *cb)
56
{
78
- struct grep_opt *opt = &grep_defaults;
57
+ struct grep_opt *opt = cb;
58
const char *slot;
59
60
if (userdiff_config(var, value) < 0)
61
return -1;
62
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
-
63
if (!strcmp(var, "grep.extendedregexp")) {
64
opt->extended_regexp_option = git_config_bool(var, value);
65
return 0;
@@ -134,14 +105,10 @@ int grep_config(const char *var, const char *value, void *cb)
105
return 0;
106
}
107
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
- */
108
void grep_init(struct grep_opt *opt, struct repository *repo)
109
{
144
- *opt = grep_defaults;
110
+ struct grep_opt blank = GREP_OPT_INIT;
111
+ memcpy(opt, &blank, sizeof(*opt));
112
113
opt->repo = repo;
114
opt->pattern_tail = &opt->pattern_list;
grep.h
+21
@@ -179,6 +179,27 @@ struct grep_opt {
179
void *output_priv;
180
};
181
182
+#define GREP_OPT_INIT { \
183
+ .relative = 1, \
184
+ .pathname = 1, \
185
+ .max_depth = -1, \
186
+ .pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED, \
187
+ .colors = { \
188
+ [GREP_COLOR_CONTEXT] = "", \
189
+ [GREP_COLOR_FILENAME] = GIT_COLOR_MAGENTA, \
190
+ [GREP_COLOR_FUNCTION] = "", \
191
+ [GREP_COLOR_LINENO] = GIT_COLOR_GREEN, \
192
+ [GREP_COLOR_COLUMNNO] = GIT_COLOR_GREEN, \
193
+ [GREP_COLOR_MATCH_CONTEXT] = GIT_COLOR_BOLD_RED, \
194
+ [GREP_COLOR_MATCH_SELECTED] = GIT_COLOR_BOLD_RED, \
195
+ [GREP_COLOR_SELECTED] = "", \
196
+ [GREP_COLOR_SEP] = GIT_COLOR_CYAN, \
197
+ }, \
198
+ .only_matching = 0, \
199
+ .color = -1, \
200
+ .output = std_output, \
201
+}
202
+
203
int grep_config(const char *var, const char *value, void *);
204
void grep_init(struct grep_opt *, struct repository *repo);
205
void grep_commit_pattern_type(enum grep_pattern_type, struct grep_opt *opt);