From: Junio C Hamano Date: Thu, 18 Jun 2015 20:05:55 GMT Subject: Re: [PATCH] format-patch: introduce format.outputDirectory configuration Message-ID: In-Reply-To: <20150618195751.GA14550@peff.net> 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.