Hi,

it is a very good idea to read the information which remote tracks which 
branch from the config, i.e. if you branch "refs/remotes/hallo", to look 
if there is any remote information for that local ref.

On Mon, 5 Mar 2007, Paolo Bonzini wrote:

quoted text
> diff --git a/builtin-branch.c b/builtin-branch.c
> index d371849..f39759b 100644
> --- a/builtin-branch.c
> +++ b/builtin-branch.c
> @@ -12,7 +12,7 @@
>  #include "builtin.h"
>  
>  static const char builtin_branch_usage[] =
> -  "git-branch [-r] (-d | -D) <branchname> | [-l] [-f] <branchname> [<start-point>] | (-m | -M) [<oldbranch>] <newbranch> | [--color | --no-color] [-r | -a] [-v [--abbrev=<length> | --no-abbrev]]";
> +  "git-branch [-r] (-d | -D) <branchname> | [--track | --no-track] [-l] [-f] <branchname> [<start-point>] | (-m | -M) [<oldbranch>] <newbranch> | [--color | --no-color] [-r | -a] [-v [--abbrev=<length> | --no-abbrev]]";
>  
>  #define REF_UNKNOWN_TYPE    0x00
>  #define REF_LOCAL_BRANCH    0x01
> @@ -308,14 +308,105 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev)
>  	free_ref_list(&ref_list);
>  }
>  
> +

extra empty line.

quoted text
> +static void get_remote_branch_name(const char* value)
> +{
> +	int len_first = -1, len_second = -1;
> +	if (*value == '+')
> +		value++;
> +

quoted text
> +	/* Try an exact match first.  */
> +	sscanf(value, "refs/%*[^:]:%n", &len_first);

This is the first time I saw that sscanf format type. How portable is it?

quoted text
> +	if (len_first != -1
> +	    && !strcmp(value + len_first, start_ref)) {

I really would rather do

	const char *p;
	if (!prefixcmp(value, "refs/") && (p = strchr(value, ':')) &&
			!strcmp(p + 1, start_ref)) {

quoted text
> +		/* Truncate the value before the colon.  */
> +		asprintf(&config_repo, "%.*s", len_first - 1, value);

asprintf() is a GNU extension. I guess it is better to just

	config_repo = xstrdup(value);
	config_repo[p - value] = '\0';


quoted text
> +		return;
> +	}
> +
> +	/* Try with a wildcard match now.  */
> +	sscanf(value, "refs/%*[^/]/*:%nrefs/remotes/%*[^/]/*%n",
> +	       &len_first, &len_second);
> +	if (len_first != -1 && len_second != -1
> +	    && (len_second - 2) - len_first == remote_len + 13
> +	    && !strncmp(value + len_first, start_ref, remote_len + 13)) {
> +		/* Replace the star with the remote branch name.  */
> +		asprintf(&config_repo, "%.*s%s",
> +			 len_first - 3, value,
> +			 start_ref + remote_len + 13);

Same here:

	else if (!prefixcmp(value, "refs/") && (p = strchr(value, ':')) &&
			!memcmp(p + 1, start_ref, remote_len) &&
			!strcmp(p + 1 + remote_len, "/*")) {
		config_repo = xstrdup(value);
		config_repo[p - value] = '\0';
	}

BTW I prefer to skip the curly brackets when there is only one statement 
in the block.

Just to be on the safe side, you might want to check here if there are 
more than one remotes "tracking" into start_ref. However, it might not be 
relevant in practice.

quoted text
> +	}
> +}
> +
> +static int get_remote_config(const char* key, const char* value)
> +{
> +        if (!prefixcmp(key, "remote.") &&
> +            !strncmp(key + 7, start_ref + 13, remote_len)) {
> +		if (config_track == -1
> +		    && !strcmp(key + 7 + remote_len, ".trackintolocalbranches"))

FWIW I don't think .trackIntoLocalBranches" is needed. Opinions?

quoted text
> +static void register_pull(const char *name, const char *real_ref, int track)

Maybe call it "set_branch_defaults()"?

quoted text
> +			die("what a long branch name you have!");
> +			goto out;

die() does not return, so no need to "goto out;"... But then, it might be 
nicer to return -1, i.e.

			ret = error("what a long branch name you have!");

and saying
		if (!ret) {
			git_config_set(key, value);
			...
			printf("Branch %s setup...
		}
	}
	if (repo_config)
		free(repo_config);
	return ret;

But I see you made it return "void", so you can skip the "return ret;".

quoted text
> @@ -333,7 +424,9 @@ static void create_branch(const char *name, const char *start_name,
>  	if (start_sha1)
>  		/* detached HEAD */
>  		hashcpy(sha1, start_sha1);
> -	else if (get_sha1(start_name, sha1))
> +	else if (dwim_ref(start_name, strlen(start_name), sha1, &real_ref) > 1)
> +		die("Ambiguous object name: '%s'.", start_name);

I know, I should have said that earlier, but I just found out myself: We 
have a config variable core.warnambiguousrefs, and maybe we should _not_ 
complain and set the defaults when the global variable warn_ambiguous_refs 
is 0.

Ciao,
Dscho

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html