Re: [PATCH] format-patch: introduce format.outputDirectory configuration

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

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Jeff King <peff@xxxxxxxx> writes:
>
>>> This change looks ugly and unnecessary.  All the machinery after and
>>> including the point set_outdir() is called, including reopen_stdout(),
>>> work on output_directory variable and only that variable.
>>> 
>>> Wouldn't it work equally well to have
>>> 
>>> 	if (!output_directory)
>>>         	output_directory = config_output_directory;
>>> 
>>> before a call to set_outdir() is made but after the configuration is
>>> read (namely, soon after parse_options() returns), without making
>>> any change to this function?
>>
>> Don't we load the config before parsing options here? In that case, we
>> can use our usual strategy to just set output_directory (which is
>> already a static global) from the config callback, and everything Just
>> Works.
>>
>> We do have to bump the definition of output_directory up above the
>> config callback, like so (while we are here, we might also want to
>> drop the unnecessary static initializers, which violate our style guide):
>
> You would also need to remove the "oh you gave me -o twice?" check,
> and change the semantics to "later -o overrides an earlier one",
> wouldn't you?  Otherwise you would never be able to override what
> you read from the config, I am afraid.

By the way, I actually think "later -o overrides an earlier one" is
a good change by itself, regardless of this new configuration.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]