Junio C Hamano <gitster@xxxxxxxxx> writes: > Matthieu Moy <Matthieu.Moy@xxxxxxx> writes: > >> diff --git a/builtin/config.c b/builtin/config.c >> index 000d27c..ecfceca 100644 >> --- a/builtin/config.c >> +++ b/builtin/config.c >> @@ -316,7 +316,7 @@ static void get_color(const char *def_color) >> >> static int get_colorbool_found; >> static int get_diff_color_found; >> -static int get_color_ui_found; >> +static int get_color_ui_found = GIT_COLOR_AUTO; > > It is curious to notice that we have these three and only one is > initialized to the new default value, while the other two get -1 > at the beginning of get_colorbool(). Right. The meaning of the _found suffix is clear for the first two, but not the last. > I wonder if it would be cleaner to statically initialize all three > to -1 here, drop the assignment of -1 to two of them from the > beginning of get_colorbool(), and then have a final fallback inside > the want_color() call itself, i.e. I've left the assignments within the function (I like the initialisation right before usage, I don't have to worry about how many times the function is called then), but I've added a patch that initializes get_color_ui_found to -1 like the others, and does essentially this: > get_colorbool_found = want_color(get_colorbool_found < 0 > ? GIT_COLOR_AUTO > : get_colorbool_found); Except I've made it a separate if statement. Then PATCH 2/2 is really crystal clear. Reroll comming, with an improved commit message that should adress the points in the other message. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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