From: Junio C Hamano Date: Wed, 15 May 2013 16:09:22 GMT Subject: Re: [PATCH v2] make color.ui default to 'auto' Message-ID: <7vtxm4cjil.fsf@alter.siamese.dyndns.org> In-Reply-To: <1368624095-15738-1-git-send-email-Matthieu.Moy@imag.fr> Matthieu Moy 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(). 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. get_colorbool_found = want_color(get_colorbool_found < 0 ? GIT_COLOR_AUTO : get_colorbool_found); so that it is clear that -1 consistently mean "We haven't read any value from the configuration file for this variable", instead of making get_color_ui_found mean slightly different thing (the value read from the configuration; GIT_COLOR_AUTO means we cannot tell if we saw this variable or the user specified auto) from the other two (the value read from the configuration; -1 means we did not find any).