Matthieu Moy <matthieu....@grenoble-inp.fr> writes:
> Junio C Hamano <gits...@pobox.com> writes:
>> Matthieu Moy <matthieu....@imag.fr> 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.
Yeah, sounds good.
> Reroll comming, with an improved commit message that should adress the
> points in the other message.
Hmm, I don't see much improvement in the message, though. It seems
to talk about "may not discover", "live with", "a few people", and
"they can easily", none of which should be there.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html