Re: [PATCH v2 6/8] grep API: call grep_config() after grep_init()
From: Junio C Hamano <hidden>
Date: 2021-11-12 17:33:04
Ævar Arnfjörð Bjarmason [off-list ref] writes:
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.
OK, so we did this because we wanted to be able to
1. call grep_config() only once to populate the template;
2. call grep_init() more than once, and match the grep_opt to
what the config wanted, without having to call grep_config()
once per grep_init() invocation.
3. each invocation of grep_init() in 2. may be followed by
parse_options() to further tweak grep_opt.
And now we instead have to do
1. call grep_init()
2. call grep_config()
3. parse_options() to tweak
for each instance of grep_opt, which is much more common.
OK.
quoted hunk
diff --git a/builtin/log.c b/builtin/log.c index f75d87e8d7f..bfddacdfa6c 100644 --- a/builtin/log.c +++ b/builtin/log.c@@ -505,8 +505,6 @@ static int git_log_config(const char *var, const char *value, void *cb) return 0; } - if (grep_config(var, value, cb) < 0) - return -1;
This used to tweak the "default template", which we no longer use, so can go? And in its place ...
quoted hunk
if (git_gpg_config(var, value, cb) < 0) return -1; return git_diff_ui_config(var, value, cb);@@ -521,6 +519,8 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix) git_config(git_log_config, NULL); repo_init_revisions(the_repository, &rev, prefix); + git_config(grep_config, &rev.grep_filter); +
... each command in the "log" family tweaks the grep_opt used for real from the configuration.
quoted hunk
rev.diff = 1; rev.simplify_history = 0; memset(&opt, 0, sizeof(opt));@@ -635,6 +635,8 @@ int cmd_show(int argc, const char **argv, const char *prefix) memset(&match_all, 0, sizeof(match_all)); repo_init_revisions(the_repository, &rev, prefix); + git_config(grep_config, &rev.grep_filter); +
Ditto. OK, the new pattern makes sense.
quoted hunk
diff --git a/grep.h b/grep.h index 62deadb885f..30a7dfd3294 100644 --- a/grep.h +++ b/grep.h@@ -171,12 +171,34 @@ struct grep_opt { int show_hunk_mark; int file_break; int heading; + void *caller_priv;
This is unrelated and unexplained change, isn't it?
void *priv;
void (*output)(struct grep_opt *opt, const void *data, size_t size);
void *output_priv;
};
+#define GREP_OPT_INIT { \
+ .relative = 1, \
+ .pathname = 1, \
+ .max_depth = -1, \
+ .pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED, \
+ .colors = { \
+ [GREP_COLOR_CONTEXT] = "", \
+ [GREP_COLOR_FILENAME] = "", \
+ [GREP_COLOR_FUNCTION] = "", \
+ [GREP_COLOR_LINENO] = "", \
+ [GREP_COLOR_COLUMNNO] = "", \
+ [GREP_COLOR_MATCH_CONTEXT] = GIT_COLOR_BOLD_RED, \
+ [GREP_COLOR_MATCH_SELECTED] = GIT_COLOR_BOLD_RED, \
+ [GREP_COLOR_SELECTED] = "", \
+ [GREP_COLOR_SEP] = GIT_COLOR_CYAN, \
+ }, \
+ .only_matching = 0, \
+ .color = -1, \
+ .output = std_output, \
+}Other than the mysterious caller_priv bit, the change makes sense to me. Thanks.