From: Jeff King <peff@peff.net>
Date: Fri, 22 Mar 2013 14:56:40 GMT
Subject: Re: [PATCH 5/6] remote.c: introduce remote.pushdefault
Message-ID: <20130322145640.GB3083@sigill.intra.peff.net>
In-Reply-To: <1363938756-13722-6-git-send-email-artagnon@gmail.com>

On Fri, Mar 22, 2013 at 01:22:35PM +0530, Ramkumar Ramachandra wrote:

> diff --git a/remote.c b/remote.c
> index 185ac11..bdb542c 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -350,6 +350,11 @@ static int handle_config(const char *key, const char *value, void *cb)
>  	const char *subkey;
>  	struct remote *remote;
>  	struct branch *branch;
> +	if (!prefixcmp(key, "remote.")) {
> +		if (!strcmp(key + 7, "pushdefault"))
> +			if (git_config_string(&pushremote_name, key, value))
> +				return -1;
> +	}
>  	if (!prefixcmp(key, "branch.")) {
>  		name = key + 7;
>  		subkey = strrchr(name, '.');

I was going to say "shouldn't we return 0" here, both on successful read
of pushdefault, but also just when we have remote.*. But the answer is
"yes" to the first one (we know we have handled the key), but "no" to
the second, because we end up parsing remote.*.* later.

So I think this should at least be:

  if (!prefixcmp(key, "remote.")) {
          if (!strcmp(key + 7, "pushdefault"))
                  return git_config_string(&pushremote_name, key, value));
          /* do not return; we handle other remote.* below */
  }

but also possibly just move it with the other remote parsing, like:

diff --git a/remote.c b/remote.c
index 02e6c4c..d3d740a 100644
--- a/remote.c
+++ b/remote.c
@@ -388,9 +388,16 @@ static int handle_config(const char *key, const char *value, void *cb)
 			add_instead_of(rewrite, xstrdup(value));
 		}
 	}
+
 	if (prefixcmp(key,  "remote."))
 		return 0;
 	name = key + 7;
+
+	/* Handle any global remote.* variables */
+	if (!strcmp(name, "pushdefault"))
+		return git_config_string(&pushremote_name, key, value);
+
+	/* Now handle any remote.NAME.* variables */
 	if (*name == '/') {
 		warning("Config remote shorthand cannot begin with '/': %s",
 			name);

-Peff