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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:05:22

Alexander Kuleshov [off-list ref] writes:
quoted hunk
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?

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

From: Jeff King <hidden>
Date: 2016-06-15 23:05:22

On Thu, Jun 18, 2015 at 10:13:37AM -0700, Junio C Hamano wrote:
quoted
-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?
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):
diff --git a/builtin/log.c b/builtin/log.c
index e67671e..77c06f7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -37,6 +37,10 @@ static int use_mailmap_config;
 static const char *fmt_patch_subject_prefix = "PATCH";
 static const char *fmt_pretty;
 
+static FILE *realstdout = NULL;
+static const char *output_directory = NULL;
+static int outdir_offset;
+
 static const char * const builtin_log_usage[] = {
 	N_("git log [<options>] [<revision-range>] [[--] <path>...]"),
 	N_("git show [<options>] <object>..."),
@@ -752,14 +756,12 @@ 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(&output_directory, var, value);
 
 	return git_log_config(var, value, cb);
 }
 
-static FILE *realstdout = NULL;
-static const char *output_directory = NULL;
-static int outdir_offset;
-
 static int reopen_stdout(struct commit *commit, const char *subject,
 			 struct rev_info *rev, int quiet)
 {
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help