Johan Herland <johan@xxxxxxxxxxx> writes: > diff --git a/diff.c b/diff.c > index 08aaa47..20fe02c 100644 > --- a/diff.c > +++ b/diff.c > @@ -45,6 +45,17 @@ static char diff_colors[][COLOR_MAXLEN] = { > GIT_COLOR_NORMAL, /* FUNCINFO */ > }; > > +static void init_default_diff_options() > +{ > + static int initialized = 0; > + if (initialized) > + return; > + > + default_diff_options.dirstat_percent = 3; > + > + initialized = 1; > +} This smells fishy on two counts. . The rest of the diff machinery is designed to be callable multiple times by calling diff_setup(), and there should be no place for any call-once function like this one. . Why is dirstat-percent _so_ special that it is the only one that has to be initialized this way, when the function name implies that this is the central place to control the initialization of all diff related options? > @@ -114,6 +125,44 @@ int git_diff_ui_config(const char *var, const char *value, void *cb) > return git_diff_basic_config(var, value, cb); > } > > +static void dirstat_opt_args(struct diff_options *options, const char *args) > +{ > + const char *p = args; > + while (*p) { > + if (!prefixcmp(p, "changes")) { > + ... > + } > + } > +} Please move this part to the previous patch in your reroll. This helper is what the previous patch should have been written with. -- 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