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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> On Mon, Apr 17, 2017 at 07:27:16PM -0700, Junio C Hamano wrote:
>
>> > @@ -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.

Yeah, I didn't see the end result was a single bit (unsigned :1).
This separation is OK.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]