Re: [PATCH v2 1/3] config: prepare to pass more info in git_config_with_options()

From: Jeff King <peff@peff.net>
Date: Apr 18, 2017, 03:55 UTC
Message-ID: <20170418035509.6zd4kxtfggei5yaj@sigill.intra.peff.net>
In-Reply-To: <xmqqd1cah2m3.fsf@gitster.mtv.corp.google.com>

On Mon, Apr 17, 2017 at 07:27:16PM -0700, Junio C Hamano wrote:

Show 17 quoted lines

> > @@ -81,7 +82,7 @@ static struct option builtin_config_options[] = {
> >  	OPT_GROUP(N_("Other")),
> >  	OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
> >  	OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
> > -	OPT_BOOL(0, "includes", &respect_includes, N_("respect include directives on lookup")),
> > +	OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")),
> 
> It would be more in line with what the log message advertised if you
> did
> 
> 	static struct config_options config_options = {
> 		-1, /* .respect_includes: unspecified */
> 	};
> 
> 	OPT_BOOL(0, "includes", &config_options.respect_includes, N_("...")),
> 
> no?

I think I like the split between the option-value here and the "final" value that goes into config_options.respect_includes. Because we actually munge it later based on the given-config value anyway.

So I agree this makes the diff larger than it might need to be, but I think the end result is a bit nicer.

-Peff