Re: [PATCH 3/3] builtin-merge: add support for default merge options

Jay Soffian <jaysoffian@gmail.com> writes:

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?