Re: [PATCH 3/3] builtin-merge: add support for default merge options
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:20
Jay Soffian [off-list ref] writes:
quoted hunk
@@ -838,6 +847,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) if (is_null_sha1(head)) head_invalid = 1; + git_config(git_merge_config_default, NULL); git_config(git_merge_config, NULL);
The placement of this comes before parse_options(), just like the part
that slurps "branch.*.mergeoptions", so it can be overridden by the
command line just like "branch.*.mergeoptions" can, which is good.
When you are on branch "frotz", your config have both merge.options and
branch.frotz.mergeoptions, and you give some other options from the
command line, how should they interact? I'd expect the branch.*.options
to take effect, ignoring merge.options entirely.
I think the right way to structure this is to change the code in
git_merge_config() that accepts "branch.*.mergeoptions" to just store a
xstrdup() pointer away, add a similar thing in the same function for the
new "merge.options" variable. Get rid of your git_merge_config_default
function that forces git_config() to iterate over the same config file one
more time. And after the config parser returns, run the parse_options
only once.
In other words, the overall code structure would look like this:
static char *options_from_config;
static int options_from_config_taken_from_branch_config;
static int git_merge_config(...)
{
if (branch && !prefixcmp(k, "branch.") ... ) {
/*
* We may have found merge.options first;
* free it and override it with the value of
* branch.*.mergeoptions for the current branch
* we just found.
*/
free(options_from_config);
options_from_config_taken_from_branch_config = 1;
options_from_config = xstrdup(value);
return 0;
}
if (!strcmp(k, "merge.options")) {
/*
* Do not override branch.*.mergeoptions for the
* current branch if we already found one.
*/
if (!options_from_config_taken_from_branch_config)
options_from_config = xstrdup(value);
return 0;
}
...
}
int cmd_merge(...)
{
...
git_config(git_merge_config, NULL);
if (options_from_config)
/*
* There is a "prime" options given in
* the configuration file. Parse it.
*/
git_config_option_string(builtin_merge_options, ...,
options_from_config);
...
argc = parse_options(argc, argv, builtin_merge_options,...);
...
}
If for some reason you would want to have cumulative options across
branch.*.merge, merge.options and the command line, then you would instead
keep two separate strings, and call git_config_option_string() for both of
them, before processing the real command line options.
Hmm?