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

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

[Alexander Kuleshov]

We can pass -o/--output-directory to the format-patch command to
store patches not in the working directory. This patch introduces
format.outputDirectory configuration option for same purpose.

The case of usage of this configuration option can be convinience
to not pass everytime -o/--output-directory if an user has pattern
to store all patches in the /patches directory for example.

The format.outputDirectory has lower priority than command line
option, so if user will set format.outputDirectory and pass the
command line option, a result will be stored in a directory that
passed to command line option.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 Documentation/config.txt           |  4 ++++
 Documentation/git-format-patch.txt |  6 +++++-
 builtin/log.c                      |  8 ++++++++
 t/t4014-format-patch.sh            | 18 ++++++++++++++++++
 4 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e159fe5..4f991b6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1262,6 +1262,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.
+
 filter.<driver>.clean::
 	The command which is used to convert the content of a worktree
 	file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 0dac4e9..38ddd76 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -57,7 +57,11 @@ The names of the output files are printed to standard
 output, unless the `--stdout` option is specified.
 
 If `-o` is specified, output files are created in <dir>.  Otherwise
-they are created in the current working directory.
+they are created in the current working directory. The default path
+can be set with the seting 'format.outputDirectory' configuration option.
+If `-o` is specified and 'format.outputDirectory' is set, output files
+will be stored in a <dir> that passed to `-o`. When 'format.outputDirectory'
+is set to get default behaviour back is to pass './' to the `-o`.
 
 By default, the subject of a single patch is "[PATCH] " followed by
 the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index 78b3e2c..fc26360 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -688,6 +688,8 @@ enum {
 	COVER_AUTO
 };
 
+static const char *config_output_directory = NULL;
+
 static int git_format_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "format.headers")) {
@@ -758,6 +760,9 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
 		return 0;
 	}
+	if (!strcmp(var, "format.outputdirectory")) {
+		return git_config_string(&config_output_directory, var, value);
+	}
 
 	return git_log_config(var, value, cb);
 }
@@ -1368,6 +1373,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (rev.show_notes)
 		init_display_notes(&rev.notes_opt);
 
+	if (!output_directory && !use_stdout)
+		output_directory = config_output_directory;
+
 	if (!use_stdout)
 		output_directory = set_outdir(prefix, output_directory);
 	else
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 890db11..613e2cc 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -40,6 +40,24 @@ test_expect_success setup '
 
 '
 
+test_expect_success "format-patch format.outputDirectory option" '
+	git config format.outputDirectory "patches/" &&
+	git format-patch master..side &&
+	cnt=$(ls | wc -l) &&
+	test $cnt = 3 &&
+	test_config format.outputDirectory "patches/" &&
+	git config --unset format.outputDirectory
+'
+
+test_expect_success "format-patch format.outputDirectory overwritten with -o" '
+	rm -rf "patches" &&
+	git config format.outputDirectory "patches/" &&
+	git format-patch master..side -o "." &&
+	test_must_fail ls patches/ &&
+	test_config format.outputDirectory "patches/" &&
+	git config --unset format.outputDirectory
+'
+
 test_expect_success "format-patch --ignore-if-in-upstream" '
 
 	git format-patch --stdout master..side >patch0 &&
-- 
2.4.4.727.g5c3049e.dirty

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

[Eric Sunshine]

On Fri, Jun 19, 2015 at 2:28 PM, Alexander Kuleshov
<kuleshovmail@gmail.com> wrote:
> We can pass -o/--output-directory to the format-patch command to
> store patches not in the working directory. This patch introduces
> format.outputDirectory configuration option for same purpose.
>
> The case of usage of this configuration option can be convinience

s/convinience/convenience/

> to not pass everytime -o/--output-directory if an user has pattern

s/everytime/every time/

> to store all patches in the /patches directory for example.
>
> The format.outputDirectory has lower priority than command line
> option, so if user will set format.outputDirectory and pass the
> command line option, a result will be stored in a directory that
> passed to command line option.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 0dac4e9..38ddd76 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -57,7 +57,11 @@ The names of the output files are printed to standard
>  output, unless the `--stdout` option is specified.
>
>  If `-o` is specified, output files are created in <dir>.  Otherwise
> -they are created in the current working directory.
> +they are created in the current working directory. The default path
> +can be set with the seting 'format.outputDirectory' configuration option.

s/seting/setting/ or s/seting//

> +If `-o` is specified and 'format.outputDirectory' is set, output files
> +will be stored in a <dir> that passed to `-o`. When 'format.outputDirectory'
> +is set to get default behaviour back is to pass './' to the `-o`.

s/set/set,/

>  By default, the subject of a single patch is "[PATCH] " followed by
>  the concatenation of lines from the commit message up to the first blank
> diff --git a/builtin/log.c b/builtin/log.c
> index 78b3e2c..fc26360 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -688,6 +688,8 @@ enum {
>         COVER_AUTO
>  };
>
> +static const char *config_output_directory = NULL;
> +
>  static int git_format_config(const char *var, const char *value, void *cb)
>  {
>         if (!strcmp(var, "format.headers")) {
> @@ -758,6 +760,9 @@ static int git_format_config(const char *var, const char *value, void *cb)
>                 config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
>                 return 0;
>         }
> +       if (!strcmp(var, "format.outputdirectory")) {
> +               return git_config_string(&config_output_directory, var, value);
> +       }

Style: Unnecessary braces.

>         return git_log_config(var, value, cb);
>  }
> @@ -1368,6 +1373,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>         if (rev.show_notes)
>                 init_display_notes(&rev.notes_opt);
>
> +       if (!output_directory && !use_stdout)
> +               output_directory = config_output_directory;
> +
>         if (!use_stdout)
>                 output_directory = set_outdir(prefix, output_directory);
>         else
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 890db11..613e2cc 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -40,6 +40,24 @@ test_expect_success setup '
>
>  '
>
> +test_expect_success "format-patch format.outputDirectory option" '
> +       git config format.outputDirectory "patches/" &&

If you're using test_config (which you do below), then drop the above
line and replace it with the test_config line.

> +       git format-patch master..side &&
> +       cnt=$(ls | wc -l) &&
> +       test $cnt = 3 &&
> +       test_config format.outputDirectory "patches/" &&
> +       git config --unset format.outputDirectory

Move the test_config line to the top of the test and get rid of the
'git config --unset' line since test_config will unset the
configuration automatically.

> +'
> +
> +test_expect_success "format-patch format.outputDirectory overwritten with -o" '
> +       rm -rf "patches" &&
> +       git config format.outputDirectory "patches/" &&

Ditto: Place test_config line here and get rid of above line.

> +       git format-patch master..side -o "." &&
> +       test_must_fail ls patches/ &&

Don't use test_must_fail for non-git commands. Instead, use '!'.
However, in this case, using:

    test_path_is_missing patches &&

would make the intent more clear.

> +       test_config format.outputDirectory "patches/" &&
> +       git config --unset format.outputDirectory

Ditto: Move test_config higher and drop 'git config --unset'.

> +'
> +
>  test_expect_success "format-patch --ignore-if-in-upstream" '
>
>         git format-patch --stdout master..side >patch0 &&
> --
> 2.4.4.727.g5c3049e.dirty

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

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Fri, Jun 19, 2015 at 2:28 PM, Alexander Kuleshov
> <kuleshovmail@gmail.com> wrote:
>> We can pass -o/--output-directory to the format-patch command to
>> store patches not in the working directory. This patch introduces
>> format.outputDirectory configuration option for same purpose.
>>
>> The case of usage of this configuration option can be convinience
>
> s/convinience/convenience/
>
>> to not pass everytime -o/--output-directory if an user has pattern
>
> s/everytime/every time/
>
>> to store all patches in the /patches directory for example.
>>
>> The format.outputDirectory has lower priority than command line
>> option, so if user will set format.outputDirectory and pass the
>> command line option, a result will be stored in a directory that
>> passed to command line option.
>>
>> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
>> ---

Looks like there were mostly editorial niggles and no fundamental
flaws in the design of the patch; it is somewhat a shame to make all
the efforts go to waste.  Will we be seeing an update soon?

We are deep in the pre-release feature freeze, so unless you are
participating in regression fixes it is a good time to plan for the
next cycle.

Thanks.

>> +static const char *config_output_directory = NULL;

s/ = NULL;/;/ (do rely on BSS clearing the static variables).

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

[Eric Sunshine]

On Mon, Sep 21, 2015 at 6:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>> On Fri, Jun 19, 2015 at 2:28 PM, Alexander Kuleshov
>> <kuleshovmail@gmail.com> wrote:
>>> We can pass -o/--output-directory to the format-patch command to
>>> store patches not in the working directory. This patch introduces
>>> format.outputDirectory configuration option for same purpose.
>>>
>>> The case of usage of this configuration option can be convinience
>>
>> s/convinience/convenience/
>>
>>> to not pass everytime -o/--output-directory if an user has pattern
>>
>> s/everytime/every time/
>>
>>> to store all patches in the /patches directory for example.
>>>
>>> The format.outputDirectory has lower priority than command line
>>> option, so if user will set format.outputDirectory and pass the
>>> command line option, a result will be stored in a directory that
>>> passed to command line option.
>>>
>>> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
>>> ---
>
> Looks like there were mostly editorial niggles and no fundamental
> flaws in the design of the patch; it is somewhat a shame to make all
> the efforts go to waste.  Will we be seeing an update soon?

Looking at the new test a bit more closely...

    test_expect_success "format-patch format.outputDirectory option" '
        git config format.outputDirectory "patches/" &&
        git format-patch master..side &&
        cnt=$(ls | wc -l) &&
        test $cnt = 3 &&
        test_config format.outputDirectory "patches/" &&
        git config --unset format.outputDirectory
    '

I'm wondering what it's really testing. I presume that it wanted to
count the number of files in the 'patches/' directory, however, the
'ls' is being invoked in the test trash directory instead. It turns
out that the trash directory has three entries at this point, so the
test succeeds, but entirely by accident.

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Looks like there were mostly editorial niggles and no fundamental
> flaws in the design of the patch; it is somewhat a shame to make all
> the efforts go to waste.  Will we be seeing an update soon?

Second ping as I am going through the what's cooking reports and
trying to decide which topics listed in [Stalled] state need to be
discarded from my tree.

Not that my dropping a topic from 'pu' means very much (a dropped
topic can still be submitted and requeued after all), even if you
are no longer interested on the topic, hearing from you would help
others who may be interested in helping the topic to completion.

Thanks.

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

[Stephen P. Smith]

From: Alexander Kuleshov <kuleshovmail@gmail.com>

We can pass -o/--output-directory to the format-patch command to
store patches not in the working directory. This patch introduces
format.outputDirectory configuration option for same purpose.

The case of usage of this configuration option can be convinience
to not pass everytime -o/--output-directory if an user has pattern
to store all patches in the /patches directory for example.

The format.outputDirectory has lower priority than command line
option, so if user will set format.outputDirectory and pass the
command line option, a result will be stored in a directory that
passed to command line option.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
Signed-off-by: Stephen P. Smith <ischis2@cox.net>
---

Notes:
    Re-rolled patch by following review comments in:
    
    http://article.gmane.org/gmane.comp.version-control.git/272199
    http://article.gmane.org/gmane.comp.version-control.git/278354
    http://article.gmane.org/gmane.comp.version-control.git/278365
    
    Changes include:
    * Specifying the patches directory as an argument to ls
    * Not initialize config_output_directory  to NULL
    * Grammar fixes in documentation
    * Use test_config rather than git config

 Documentation/config.txt           |  4 ++++
 Documentation/git-format-patch.txt |  6 +++++-
 builtin/log.c                      |  7 +++++++
 t/t4014-format-patch.sh            | 13 +++++++++++++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f617886..e92a0ee 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1243,6 +1243,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.
+
 filter.<driver>.clean::
 	The command which is used to convert the content of a worktree
 	file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index e3cdaeb..7a76594 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -57,7 +57,11 @@ The names of the output files are printed to standard
 output, unless the `--stdout` option is specified.
 
 If `-o` is specified, output files are created in <dir>.  Otherwise
-they are created in the current working directory.
+they are created in the current working directory. The default path
+can be set with the setting 'format.outputDirectory' configuration option.
+If `-o` is specified and 'format.outputDirectory' is set, output files
+will be stored in a <dir> that passed to `-o`. When 'format.outputDirectory'
+is set, to get default behaviour back is to pass './' to the `-o`.
 
 By default, the subject of a single patch is "[PATCH] " followed by
 the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index e00cea7..679ff76 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -707,6 +707,8 @@ enum {
 	COVER_AUTO
 };
 
+static const char *config_output_directory;
+
 static int git_format_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "format.headers")) {
@@ -777,6 +779,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
 		return 0;
 	}
+	if (!strcmp(var, "format.outputdirectory"))
+		return git_config_string(&config_output_directory, var, value);
 
 	return git_log_config(var, value, cb);
 }
@@ -1391,6 +1395,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (rev.show_notes)
 		init_display_notes(&rev.notes_opt);
 
+	if (!output_directory && !use_stdout)
+		output_directory = config_output_directory;
+
 	if (!use_stdout)
 		output_directory = set_outdir(prefix, output_directory);
 	else
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 646c475..5c6f128 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -40,6 +40,19 @@ test_expect_success setup '
 
 '
 
+test_expect_success "format-patch format.outputDirectory option" '
+	test_config format.outputDirectory "patches/" &&
+	git format-patch master..side &&
+	cnt=$(ls patches | wc -l) &&
+	test $cnt = 3
+'
+
+test_expect_success "format-patch format.outputDirectory overwritten with -o" '
+	test_config format.outputDirectory "patches/" &&
+	git format-patch master..side -o "." &&
+	test_path_is_missing patches/
+'
+
 test_expect_success "format-patch --ignore-if-in-upstream" '
 
 	git format-patch --stdout master..side >patch0 &&
-- 
2.7.0-rc2

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

[Eric Sunshine]

Thanks for reviving this abandoned patch. Please see review comments
below, some of which repeat comments from the previous attempt[1], and
some of which are new. Most of the new ones are minor, although there
is at least one major problem.

On Sat, Jan 9, 2016 at 9:30 PM, Stephen P. Smith <ischis2@cox.net> wrote:
> From: Alexander Kuleshov <kuleshovmail@gmail.com>
>
> We can pass -o/--output-directory to the format-patch command to
> store patches not in the working directory. This patch introduces

s/not in/in some place other than/

> format.outputDirectory configuration option for same purpose.
>
> The case of usage of this configuration option can be convinience

>From [1]: s/convinience/convenience/

> to not pass everytime -o/--output-directory if an user has pattern

Also[1]: s/everytime/every time/
or: s/everytime/each time/

> to store all patches in the /patches directory for example.
>
> The format.outputDirectory has lower priority than command line
> option, so if user will set format.outputDirectory and pass the
> command line option, a result will be stored in a directory that
> passed to command line option.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> Signed-off-by: Stephen P. Smith <ischis2@cox.net>

[1]: http://article.gmane.org/gmane.comp.version-control.git/272199

> ---
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> @@ -57,7 +57,11 @@ The names of the output files are printed to standard
>  output, unless the `--stdout` option is specified.
>
>  If `-o` is specified, output files are created in <dir>.  Otherwise
> -they are created in the current working directory.
> +they are created in the current working directory. The default path
> +can be set with the setting 'format.outputDirectory' configuration option.

s/setting//

> +If `-o` is specified and 'format.outputDirectory' is set, output files
> +will be stored in a <dir> that passed to `-o`. When 'format.outputDirectory'
> +is set, to get default behaviour back is to pass './' to the `-o`.

This is difficult to read. How about replacing these two sentences
with something like this:

    The `-o` option takes precedence over `format.outputDirectory`.
    To store patches in the current working directory even when
    `format.outputDirectory` points elsewhere, use `-o .`.

> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> @@ -40,6 +40,19 @@ test_expect_success setup '

Rather than adding new tests at the very top of the script, it's more
common to add them to the bottom or at least to insert them after
other similar tests.

> +test_expect_success "format-patch format.outputDirectory option" '

Use single- rather than double-quotes: s/"/'/

> +       test_config format.outputDirectory "patches/" &&

We can drop the unnecessary quotes around "patches".
Also, can we drop the unnecessary "/"?

> +       git format-patch master..side &&

Since this test is about verifying that the "patches" directory got
created and used, you want to be more careful about ensuring that
detritus from preceding tests won't muck up your results; for
instance, if an earlier test had also used a directory named "patches"
and had dumped 42 files there instead of the 3 expected by this test.
Therefore, you should insert "rm -fr patches &&" before the
git-format-patch invocation.

> +       cnt=$(ls patches | wc -l) &&
> +       test $cnt = 3

Periodically, we have trouble with the output of "wc -l" on Mac OS X
since the output has leading spaces. This code doesn't trip over that
problem since it doesn't quote the output, but it still feels fragile
to be comparing the it against a number using the string equality test
'='. How about using the '-eq' numeric equality test instead?

Moreover, there is no need for the temporary 'cnt' variable. Instead:

    test $(ls patches | wc -l) -eq 3 &&

Depending upon taste, you might alternately use:

    ls patches >actual &&
    test_line_count = 3 actual

which would give you more useful debugging output upon failure.

> +'
> +
> +test_expect_success "format-patch format.outputDirectory overwritten with -o" '

Use single- rather than double-qoutes: s/"/'/
Also, how about rewording it?

    'format-patch -o overrides format.outputDirectory'

> +       test_config format.outputDirectory "patches/" &&

Style: drop unnecessary quotes around "patches"
Style: drop unnecessary "/"

> +       git format-patch master..side -o "." &&

Style: drop unnecessary quotes around "."

> +       test_path_is_missing patches/

Style: drop unnecessary "/"

There is a rather severe problem with this test in that it fails
unconditionally. It wants to verify that -o takes precedence over
format.outputDirectory by checking that the directory "patches" did
not get created by git-format-patch, however, that directory already
exists since it was created by the previous test, thus
test_path_is_missing() fails. Therefore, you should insert "rm -fr
patches &&" before the git-format-patch invocation.

It also might not hurt to make the test a bit more robust by verifying
not only that the directory specified by format.outputDirectory did
not get created, but that the directory named by -o did get created,
which means giving -o an argument other than ".". So, the final test
might look like this:

    test_config format.outputDirectory patches &&
    rm -fr patches patchset &&
    git format-patch master..side -o patchset &&
    test_path_is_missing patches &&
    test_path_is_dir patchset

> +'
> --
> 2.7.0-rc2

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

[Stephen P. Smith]

From: Alexander Kuleshov <kuleshovmail@gmail.com>

We can pass -o/--output-directory to the format-patch command to store
patches in some place other than the working directory. This patch
introduces format.outputDirectory configuration option for same
purpose.

The case of usage of this configuration option can be convinience
to not pass every time -o/--output-directory if an user has pattern
to store all patches in the /patches directory for example.

The format.outputDirectory has lower priority than command line
option, so if user will set format.outputDirectory and pass the
command line option, a result will be stored in a directory that
passed to command line option.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
Signed-off-by: Stephen P. Smith <ischis2@cox.net>
---

Notes:
    Updated based on http://article.gmane.org/gmane.comp.version-control.git/283613
    
    Commit message changes:
      s/not in/in some place other than/.
      s/everytime/every time/.
    
    git-format-patch.txt change:
      s/setting//.
      applied the two sentence wording change.
    
    t4014-format-patch.sh:
      moved tests to the end of the test suite.
      added rm commands to remove directory prior to testing.
      changed directory name style issues.

 Documentation/config.txt           |  4 ++++
 Documentation/git-format-patch.txt |  6 +++++-
 builtin/log.c                      |  7 +++++++
 t/t4014-format-patch.sh            | 16 ++++++++++++++++
 4 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f617886..e92a0ee 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1243,6 +1243,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.
+
 filter.<driver>.clean::
 	The command which is used to convert the content of a worktree
 	file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index e3cdaeb..64c2803 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -57,7 +57,11 @@ The names of the output files are printed to standard
 output, unless the `--stdout` option is specified.
 
 If `-o` is specified, output files are created in <dir>.  Otherwise
-they are created in the current working directory.
+they are created in the current working directory. The default path
+can be set with the 'format.outputDirectory' configuration option.
+The `-o` option takes precedence over `format.outputDirectory`.
+To store patches in the current working directory even when
+`format.outputDirectory` points elsewhere, use `-o .`.
 
 By default, the subject of a single patch is "[PATCH] " followed by
 the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index e00cea7..679ff76 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -707,6 +707,8 @@ enum {
 	COVER_AUTO
 };
 
+static const char *config_output_directory;
+
 static int git_format_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "format.headers")) {
@@ -777,6 +779,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
 		return 0;
 	}
+	if (!strcmp(var, "format.outputdirectory"))
+		return git_config_string(&config_output_directory, var, value);
 
 	return git_log_config(var, value, cb);
 }
@@ -1391,6 +1395,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (rev.show_notes)
 		init_display_notes(&rev.notes_opt);
 
+	if (!output_directory && !use_stdout)
+		output_directory = config_output_directory;
+
 	if (!use_stdout)
 		output_directory = set_outdir(prefix, output_directory);
 	else
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 646c475..a662cb2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1445,4 +1445,20 @@ test_expect_success 'From line has expected format' '
 	test_cmp from filtered
 '
 
+test_expect_success 'format-patch format.outputDirectory option' '
+	test_config format.outputDirectory patches &&
+	rm -fr patches &&
+	git format-patch master..side &&
+	ls patches >actual &&
+	test_line_count = 3 actual
+'
+
+test_expect_success 'format-patch -o overrides format.outputDirectory' '
+	test_config format.outputDirectory patches &&
+	rm -fr patches patchset &&
+	git format-patch master..side -o patchset &&
+	test_path_is_missing patches &&
+	test_path_is_dir patchset
+'
+
 test_done
-- 
2.7.0-rc2

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

[Eric Sunshine]

On Sun, Jan 10, 2016 at 7:30 PM, Stephen P. Smith <ischis2@cox.net> wrote:
> We can pass -o/--output-directory to the format-patch command to store
> patches in some place other than the working directory. This patch
> introduces format.outputDirectory configuration option for same
> purpose.
>
> The case of usage of this configuration option can be convinience
> to not pass every time -o/--output-directory if an user has pattern
> to store all patches in the /patches directory for example.
>
> The format.outputDirectory has lower priority than command line
> option, so if user will set format.outputDirectory and pass the
> command line option, a result will be stored in a directory that
> passed to command line option.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> Signed-off-by: Stephen P. Smith <ischis2@cox.net>
> ---
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> @@ -1445,4 +1445,20 @@ test_expect_success 'From line has expected format' '
> +test_expect_success 'format-patch format.outputDirectory option' '
> +       test_config format.outputDirectory patches &&
> +       rm -fr patches &&
> +       git format-patch master..side &&
> +       ls patches >actual &&
> +       test_line_count = 3 actual
> +'

Now that you've moved the new tests to the bottom of the script, this
test fails. This is because, at this point, there are 18 commits in
the range 'master..side', not 3 as when the test was at the top of the
script. You could change the 3 to an 18, however, that would be
fragile: if someone inserts or modifies tests above this one, then a
hard-coded 18 might become stale. One possible fix would be:

    test $(git rev-list master..side | wc -l) -eq $(ls patches)

You could also take the stance that you're not so much interested in
the number of patches in the range 'master..side' but rather you
merely care about the fact that the "patches" directory got created
and some patches were deposited there. In that case, you might do this
instead:

    git format-patch -3 side &&
    ls patches >actual &&
    test_line_count = 3 actual

I don't feel strongly about it either way, but whichever approach you
choose, please do build the project and run this test script to ensure
that it succeeds before submitting v5. Thanks.

> +test_expect_success 'format-patch -o overrides format.outputDirectory' '
> +       test_config format.outputDirectory patches &&
> +       rm -fr patches patchset &&
> +       git format-patch master..side -o patchset &&
> +       test_path_is_missing patches &&
> +       test_path_is_dir patchset
> +'

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

[Eric Sunshine]

On Sun, Jan 10, 2016 at 10:55 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sun, Jan 10, 2016 at 7:30 PM, Stephen P. Smith <ischis2@cox.net> wrote:
>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>> @@ -1445,4 +1445,20 @@ test_expect_success 'From line has expected format' '
>> +test_expect_success 'format-patch format.outputDirectory option' '
>> +       test_config format.outputDirectory patches &&
>> +       rm -fr patches &&
>> +       git format-patch master..side &&
>> +       ls patches >actual &&
>> +       test_line_count = 3 actual
>> +'
>
> Now that you've moved the new tests to the bottom of the script, this
> test fails. This is because, at this point, there are 18 commits in
> the range 'master..side', not 3 as when the test was at the top of the
> script. You could change the 3 to an 18, however, that would be
> fragile: if someone inserts or modifies tests above this one, then a
> hard-coded 18 might become stale. One possible fix would be:
>
>     test $(git rev-list master..side | wc -l) -eq $(ls patches)

That would be $(ls patches | wc -l), of course.

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

[Stephen P. Smith]

From: Alexander Kuleshov <kuleshovmail@gmail.com>

We can pass -o/--output-directory to the format-patch command to store
patches in some place other than the working directory. This patch
introduces format.outputDirectory configuration option for same
purpose.

The case of usage of this configuration option can be convinience
to not pass every time -o/--output-directory if an user has pattern
to store all patches in the /patches directory for example.

The format.outputDirectory has lower priority than command line
option, so if user will set format.outputDirectory and pass the
command line option, a result will be stored in a directory that
passed to command line option.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
Signed-off-by: Stephen P. Smith <ischis2@cox.net>
---

Notes:
    Fixed bug which was found when moving the tests to the end of the 
    script by removing the hardcoded expected count and replacing with a 
    computation of the number of actual patches.

    Mailing list web interface is again not working; therefore, I don't 
    have URLs for the earlier review comments.
    
 Documentation/config.txt           |  4 ++++
 Documentation/git-format-patch.txt |  6 +++++-
 builtin/log.c                      |  7 +++++++
 t/t4014-format-patch.sh            | 15 +++++++++++++++
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f617886..e92a0ee 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1243,6 +1243,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.
+
 filter.<driver>.clean::
 	The command which is used to convert the content of a worktree
 	file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index e3cdaeb..64c2803 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -57,7 +57,11 @@ The names of the output files are printed to standard
 output, unless the `--stdout` option is specified.
 
 If `-o` is specified, output files are created in <dir>.  Otherwise
-they are created in the current working directory.
+they are created in the current working directory. The default path
+can be set with the 'format.outputDirectory' configuration option.
+The `-o` option takes precedence over `format.outputDirectory`.
+To store patches in the current working directory even when
+`format.outputDirectory` points elsewhere, use `-o .`.
 
 By default, the subject of a single patch is "[PATCH] " followed by
 the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index e00cea7..679ff76 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -707,6 +707,8 @@ enum {
 	COVER_AUTO
 };
 
+static const char *config_output_directory;
+
 static int git_format_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "format.headers")) {
@@ -777,6 +779,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
 		return 0;
 	}
+	if (!strcmp(var, "format.outputdirectory"))
+		return git_config_string(&config_output_directory, var, value);
 
 	return git_log_config(var, value, cb);
 }
@@ -1391,6 +1395,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (rev.show_notes)
 		init_display_notes(&rev.notes_opt);
 
+	if (!output_directory && !use_stdout)
+		output_directory = config_output_directory;
+
 	if (!use_stdout)
 		output_directory = set_outdir(prefix, output_directory);
 	else
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 646c475..3b99434 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1445,4 +1445,19 @@ test_expect_success 'From line has expected format' '
 	test_cmp from filtered
 '
 
+test_expect_success 'format-patch format.outputDirectory option' '
+	test_config format.outputDirectory patches &&
+	rm -fr patches &&
+	git format-patch master..side &&
+	test $(git rev-list master..side | wc -l) -eq $(ls patches | wc -l)
+'
+
+test_expect_success 'format-patch -o overrides format.outputDirectory' '
+	test_config format.outputDirectory patches &&
+	rm -fr patches patchset &&
+	git format-patch master..side -o patchset &&
+	test_path_is_missing patches &&
+	test_path_is_dir patchset
+'
+
 test_done
-- 
2.7.0-rc2

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

[Eric Sunshine]

On Tue, Jan 12, 2016 at 11:48 PM, Stephen P. Smith <ischis2@cox.net> wrote:
> From: Alexander Kuleshov <kuleshovmail@gmail.com>
>
> We can pass -o/--output-directory to the format-patch command to store
> patches in some place other than the working directory. This patch
> introduces format.outputDirectory configuration option for same
> purpose.
>
> The case of usage of this configuration option can be convinience

Mentioned several times already: s/convinience/convenience/

> to not pass every time -o/--output-directory if an user has pattern
> to store all patches in the /patches directory for example.
>
> The format.outputDirectory has lower priority than command line
> option, so if user will set format.outputDirectory and pass the
> command line option, a result will be stored in a directory that
> passed to command line option.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> Signed-off-by: Stephen P. Smith <ischis2@cox.net>
> ---
> Notes:
>     Fixed bug which was found when moving the tests to the end of the
>     script by removing the hardcoded expected count and replacing with a
>     computation of the number of actual patches.

Thanks, this version looks better. Aside from the misspelling above
and a minor comment below, this version is:

Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

>     Mailing list web interface is again not working; therefore, I don't
>     have URLs for the earlier review comments.

The full set of attempts is here [1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/272180

> diff --git a/builtin/log.c b/builtin/log.c
> @@ -707,6 +707,8 @@ enum {
>         COVER_AUTO
>  };
>
> +static const char *config_output_directory;

I don't care strongly, but I wonder why this new variable is placed
below the enum rather than being grouped with other similar variables
just above this enum. (Probably not worth a re-roll, though.)

>  static int git_format_config(const char *var, const char *value, void *cb)
>  {
>         if (!strcmp(var, "format.headers")) {

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

[Stephen P. Smith]

From: Alexander Kuleshov <kuleshovmail@gmail.com>

We can pass -o/--output-directory to the format-patch command to store
patches in some place other than the working directory. This patch
introduces format.outputDirectory configuration option for same
purpose.

The case of usage of this configuration option can be convenience
to not pass every time -o/--output-directory if an user has pattern
to store all patches in the /patches directory for example.

The format.outputDirectory has lower priority than command line
option, so if user will set format.outputDirectory and pass the
command line option, a result will be stored in a directory that
passed to command line option.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
Signed-off-by: Stephen P. Smith <ischis2@cox.net>
---

Notes:
    Fixed s/convinience/convenience/
    
    Moved 'static const char *config_output_directory;' to be with othe
    similarly typed variables.
    
    The full set of attempts is here [1].
    
    [1]: http://thread.gmane.org/gmane.comp.version-control.git/272180

 Documentation/config.txt           |  4 ++++
 Documentation/git-format-patch.txt |  6 +++++-
 builtin/log.c                      |  6 ++++++
 t/t4014-format-patch.sh            | 15 +++++++++++++++
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f617886..e92a0ee 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1243,6 +1243,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.
+
 filter.<driver>.clean::
 	The command which is used to convert the content of a worktree
 	file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index e3cdaeb..64c2803 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -57,7 +57,11 @@ The names of the output files are printed to standard
 output, unless the `--stdout` option is specified.
 
 If `-o` is specified, output files are created in <dir>.  Otherwise
-they are created in the current working directory.
+they are created in the current working directory. The default path
+can be set with the 'format.outputDirectory' configuration option.
+The `-o` option takes precedence over `format.outputDirectory`.
+To store patches in the current working directory even when
+`format.outputDirectory` points elsewhere, use `-o .`.
 
 By default, the subject of a single patch is "[PATCH] " followed by
 the concatenation of lines from the commit message up to the first blank
diff --git a/builtin/log.c b/builtin/log.c
index e00cea7..0d738d6 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -699,6 +699,7 @@ static int do_signoff;
 static const char *signature = git_version_string;
 static const char *signature_file;
 static int config_cover_letter;
+static const char *config_output_directory;
 
 enum {
 	COVER_UNSET,
@@ -777,6 +778,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
 		return 0;
 	}
+	if (!strcmp(var, "format.outputdirectory"))
+		return git_config_string(&config_output_directory, var, value);
 
 	return git_log_config(var, value, cb);
 }
@@ -1391,6 +1394,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (rev.show_notes)
 		init_display_notes(&rev.notes_opt);
 
+	if (!output_directory && !use_stdout)
+		output_directory = config_output_directory;
+
 	if (!use_stdout)
 		output_directory = set_outdir(prefix, output_directory);
 	else
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 646c475..3b99434 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1445,4 +1445,19 @@ test_expect_success 'From line has expected format' '
 	test_cmp from filtered
 '
 
+test_expect_success 'format-patch format.outputDirectory option' '
+	test_config format.outputDirectory patches &&
+	rm -fr patches &&
+	git format-patch master..side &&
+	test $(git rev-list master..side | wc -l) -eq $(ls patches | wc -l)
+'
+
+test_expect_success 'format-patch -o overrides format.outputDirectory' '
+	test_config format.outputDirectory patches &&
+	rm -fr patches patchset &&
+	git format-patch master..side -o patchset &&
+	test_path_is_missing patches &&
+	test_path_is_dir patchset
+'
+
 test_done
-- 
2.7.0-rc2

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

[Eric Sunshine]

On Wed, Jan 13, 2016 at 8:20 AM, Stephen P. Smith <ischis2@cox.net> wrote:
> From: Alexander Kuleshov <kuleshovmail@gmail.com>
>
> We can pass -o/--output-directory to the format-patch command to store
> patches in some place other than the working directory. This patch
> introduces format.outputDirectory configuration option for same
> purpose.
>
> The case of usage of this configuration option can be convenience
> to not pass every time -o/--output-directory if an user has pattern
> to store all patches in the /patches directory for example.
>
> The format.outputDirectory has lower priority than command line
> option, so if user will set format.outputDirectory and pass the
> command line option, a result will be stored in a directory that
> passed to command line option.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> Signed-off-by: Stephen P. Smith <ischis2@cox.net>
> ---
> Notes:
>     Fixed s/convinience/convenience/
>
>     Moved 'static const char *config_output_directory;' to be with othe
>     similarly typed variables.

Thanks. This version is also:

Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

(A note for future submissions of other patches: Once a person has
given a Reviewed-by:, you're welcome to include the Reviewed-by: in a
re-roll provided that the re-roll doesn't change anything which would
obviously invalidate the Reviewed-by:. In this particular case, for
instance, v6 merely fixed a couple very minor nits mentioned in my v5
review, so it would have been perfectly acceptable to include my
Reviewed-by: in v6.)

>  Documentation/config.txt           |  4 ++++
>  Documentation/git-format-patch.txt |  6 +++++-
>  builtin/log.c                      |  6 ++++++
>  t/t4014-format-patch.sh            | 15 +++++++++++++++
>  4 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index f617886..e92a0ee 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1243,6 +1243,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.
> +
>  filter.<driver>.clean::
>         The command which is used to convert the content of a worktree
>         file to a blob upon checkin.  See linkgit:gitattributes[5] for
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index e3cdaeb..64c2803 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -57,7 +57,11 @@ The names of the output files are printed to standard
>  output, unless the `--stdout` option is specified.
>
>  If `-o` is specified, output files are created in <dir>.  Otherwise
> -they are created in the current working directory.
> +they are created in the current working directory. The default path
> +can be set with the 'format.outputDirectory' configuration option.
> +The `-o` option takes precedence over `format.outputDirectory`.
> +To store patches in the current working directory even when
> +`format.outputDirectory` points elsewhere, use `-o .`.
>
>  By default, the subject of a single patch is "[PATCH] " followed by
>  the concatenation of lines from the commit message up to the first blank
> diff --git a/builtin/log.c b/builtin/log.c
> index e00cea7..0d738d6 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -699,6 +699,7 @@ static int do_signoff;
>  static const char *signature = git_version_string;
>  static const char *signature_file;
>  static int config_cover_letter;
> +static const char *config_output_directory;
>
>  enum {
>         COVER_UNSET,
> @@ -777,6 +778,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
>                 config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
>                 return 0;
>         }
> +       if (!strcmp(var, "format.outputdirectory"))
> +               return git_config_string(&config_output_directory, var, value);
>
>         return git_log_config(var, value, cb);
>  }
> @@ -1391,6 +1394,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>         if (rev.show_notes)
>                 init_display_notes(&rev.notes_opt);
>
> +       if (!output_directory && !use_stdout)
> +               output_directory = config_output_directory;
> +
>         if (!use_stdout)
>                 output_directory = set_outdir(prefix, output_directory);
>         else
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 646c475..3b99434 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1445,4 +1445,19 @@ test_expect_success 'From line has expected format' '
>         test_cmp from filtered
>  '
>
> +test_expect_success 'format-patch format.outputDirectory option' '
> +       test_config format.outputDirectory patches &&
> +       rm -fr patches &&
> +       git format-patch master..side &&
> +       test $(git rev-list master..side | wc -l) -eq $(ls patches | wc -l)
> +'
> +
> +test_expect_success 'format-patch -o overrides format.outputDirectory' '
> +       test_config format.outputDirectory patches &&
> +       rm -fr patches patchset &&
> +       git format-patch master..side -o patchset &&
> +       test_path_is_missing patches &&
> +       test_path_is_dir patchset
> +'
> +
>  test_done
> --
> 2.7.0-rc2