From: Junio C Hamano Date: Thu, 18 Jun 2015 20:06:54 GMT Subject: Re: [PATCH] format-patch: introduce format.outputDirectory configuration Message-ID: In-Reply-To: Junio C Hamano writes: > Jeff King 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.