Junio C Hamano <gitster@xxxxxxxxx> writes: > So we would need to do something like: > > - call git_log_config() first to let diff_context_default > updated from the configuration as before. find the values of > grep.* defaults at the same time, but stash it away in a > separate "struct grep_opt" (yuck); > > - call init_revisions() and let it initialize revs->grep_filter > and revs->diffopt as before; > > - copy the grep.* defaults we learned during git_log_config() to > revs->grep_filter. > > which is a bit yucky, but survivable. After thinking about it a bit more, I came to a conclusion that the configuration handling lifted from builtin/grep.c needs a much larger overhaul. The grep_config() function takes one instance of grep_opt as a 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. The codepath under discussion in this thread is a prime example why that arrangement is not always possible. - 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. The right way to arrange your configuration callback is probably to model it after how diff configuration variables are handled. You call git_config() once, and remember the values you read in set of static variables. Later, whenever you need to instantiate a grep_opt, you initialize it from these static variables. All of the above did not matter back when the code in builtin/grep.c was isolated and the configuration was never meant to be used by other subsystems. But the last two patches in this series do want to break that assumption, so grep_config() needs to be rethought. Luckily, we don't have to have this in the upcoming 1.8.0 release (it is is too late for any topic that is not a regression fix). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html