On Sun, Nov 12, 2017 at 03:00:40PM +0000, Soukaina NAIT HMID wrote:
> diff --git a/builtin/config.c b/builtin/config.c
> index 124a682d50fa8..9df2d9c43bcad 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -30,6 +30,7 @@ static int end_null;
> static int respect_includes_opt = -1;
> static struct config_options config_options;
> static int show_origin;
> +static const char *default_value;
> [...]
> + OPT_STRING(0, "default", &default_value, N_("default-value"), N_("sets default for bool/int/path/color when no value is returned from config")),
These hunks make sense. We're adding a new "--default" option that would
kick in when you try to look up a key and it isn't present.
I think we can skip the "bool/int/path/color" thing in the help string.
We would want this to kick in for every type, right? The only
constraint is that we are doing a "get" operation. It wouldn't make any
sense to use "--default" when setting a variable, listing, etc. Should
we catch these cases and return an error?
We'd also want to mention this in Documentation/git-config.txt.
> @@ -47,6 +48,7 @@ static int show_origin;
> #define ACTION_GET_COLOR (1<<13)
> #define ACTION_GET_COLORBOOL (1<<14)
> #define ACTION_GET_URLMATCH (1<<15)
> +#define ACTION_GET_COLORORDEFAULT (1<<16)
I'm not sure I understand this part, though. Providing a default should
be something that goes along with a "get" action, but isn't its own
action.
> +static void get_color_default(const char *var)
> +{
> + get_color(var, default_value);
> +}
> +
And here we're just applying --default to colors, but we'd eventually
want them for everything. I think that's fixed later in the series, so
I'll keep reading. But I'd expect a function like get_value() to be
detecting the case where we got no hits and filling in the default_value
there, as if we had read it from the config file.
-Peff
