Subject: Re: [PATCH, 4th version] git-branch: register where to merge from, when branching off a remote branch

From: Paolo Bonzini <[email-masked]>
Date: Fri, 02 Mar 2007 12:14:50 +0100

> I have a feeling that "git checkout -b" and "git checkout -B"
> should be taught to explicitly use "git branch --no-track" and
> "git branch --track" to create a new branch (currently it does
> not even use "git branch" as far as I can tell).  With your
> patch, I suspect that you have to say "git branch topic
> origin/topic" and then "git checkout topic", which means you
> made the three-step process into two steps, but you could have
> made it into one step.

Well, yes, the next part would have been to patch git-checkout.  But this one is already proving to be complicated enough.  :-)

> I'll send out an untested patch to
> git-checkout so that you can try it out in a separate message.

I saw it.  I tested your patch and it seems to work.
 
>  - (micronit) Is it true that both strlen() tests are about long
>    *branch* names?

If you consider something like "origin/next" to be a branch name, yes.

>  - (moderately serious) The code blindly trusts that
>    "refs/remotes/foo/bar" tracks "refs/heads/bar" from remote
>    named "foo", which is a bit disturbing.  With the default
>    configuration git-clone and git-remote creates, it always is
>    the case, but I suspect you might want to at least verify
>    that assumption (the user can have different settings in the
>    config), if not figuring them out by reading the existing
>    configuration yourself.

Ouch.  Absolutely right, but this means I will prepare the patch later then.

>> +	else if (dwim_ref(start_name, strlen(start_name), sha1, &real_ref))
>> +		remote = !prefixcmp(real_ref, "refs/remotes/");
> 
>  - (pure question) What happens if dwim_ref() returns more than one?

Then, real_ref is the one matching sha1.

Considering your other objection about the naming of the variable, what about enabling/disabling the tracking using remote.REMOTENAME.tracklocalsubbranches?

Paolo