Re: [PATCH] Add support for merging from upstream by default.

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 7, 2011, 01:59 UTC
Message-ID: <7vaai8wqny.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1296912681-4161-1-git-send-email-jaredhance@gmail.com>

Jared Hance <jaredhance@gmail.com> writes:

> Adds the option merge.defaultupstream to add support for merging from the
> upstream branch by default. The upstream branch is found using
> branch.[name].upstream.
> ---

Sign off?

The patch (rather your code before getting handed to your MUA) seems severely whitespace damaged.

Show 33 quoted lines

> diff --git a/builtin/merge.c b/builtin/merge.c
> index 42fff38..596febe 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -37,7 +37,7 @@ struct strategy {
>  };
>  
>  static const char * const builtin_merge_usage[] = {
> -	"git merge [options] <remote>...",
> +	"git merge [options] [<remote>...]",
>  	"git merge [options] <msg> HEAD <remote>",
>  	NULL
>  };
> @@ -58,6 +58,8 @@ static int option_renormalize;
>  static int verbosity;
>  static int allow_rerere_auto;
>  static int abort_current_merge;
> +static int default_upstream;
> +static const char *upstream_branch;
>  
>  static struct strategy all_strategy[] = {
>  	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
> @@ -519,8 +521,15 @@ static int git_merge_config(const char *k, const char *v, void *cb)
>  			      builtin_merge_usage, 0);
>  		free(buf);
>  	}
> -
> -	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
> +        else if(branch && !prefixcmp(k, "branch.") &&
> +                !prefixcmp(k + 7, branch) &&
> +                !strcmp(k + 7 + strlen(branch), ".upstream")) {
> +                return git_config_string(&upstream_branch, k, v);
> +        }

Shouldn't this be inside a single large if block that is guarded by

	if (branch && !prefixcmp(k, "branch.") && !prefixcmp(k + 7, branch))

with a small refactoring of the existing code in git_merge_config(), the first if statement? It probably is a good idea to hand that off to a small helper function as well, i.e. the first if statement in git_merge_config() becomes something like:

	status = per_branch_config(k, v, cb);
        if (status <= 0)
		return status;

and then a new helper function is defined right in front of it, perhaps

	static int per_branch_config(const char *k, const char *v, void *cb)
	{
        	const char *variable;
        	if (!branch || prefixcmp(k, "branch.")
                    || prefixcmp(k + 7, branch))
			return 1; /* ignore me */
		variable = k + 7 + strlen(branch);
                if (strcmp(variable, ".mergeoptions")) {
                	...
                        return 0; /* done */
		}
                if (strcmp(variable, ".upstream")) {
                	...
                        return 0; /* or -1 if you see an error */
		}
                return 1; /* not what I handle */
	}