From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 18 Jun 2015 17:13:37 GMT
Subject: Re: [PATCH] format-patch: introduce format.outputDirectory configuration
Message-ID: <xmqq616ley7y.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1434626280-4610-1-git-send-email-kuleshovmail@gmail.com>

Alexander Kuleshov <kuleshovmail@gmail.com> writes:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index fd2036c..8f6f7ed 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1247,6 +1247,10 @@ format.coverLetter::
>  	format-patch is invoked, but in addition can be set to "auto", to
>  	generate a cover-letter only when there's more than one patch.
>  
> +format.outputDirectory::
> +	Set a custom directory to store the resulting files instead of the
> +	current working directory.
> +

After you set this configuration variable, how would you override it
and get the default behaviour back from the command line for one
time invocation?  "-o ./"?  That needs to be documented somewhere.

Documentation/format-patch.txt must have description on -o; that
paragraph needs to mention this new configuration variable, and it
would be a good place to document the "-o ./" workaround.

> -static const char *set_outdir(const char *prefix, const char *output_directory)
> +static const char *set_outdir(const char *prefix, const char *output_directory,
> +			      const char *config_output_directory)

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?