Hi,
On Mon, 5 Mar 2007, Paolo Bonzini wrote:
> >> + /* 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?
>
> It is. At the very least it is in OpenGroup.
That is not very reassuring for me. However, I learnt something new about
sscanf(), so it was not lost on me.
> >> + /* 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';
>
> git has nfvasprintf -- I'll just use that one.
Which forces you to build a va_list first, and which is horribly
inefficient. Not that it matters for git-branch...
> > FWIW I don't think .trackIntoLocalBranches" is needed. Opinions?
>
> That's because I'd like to make it the default for me... Also, look at
> patch 3/3.
I am not only mildly opposed to switching trackIntoLocalBranches off. It
is a _very_ useful feature for new Git users, as it will (I am sure)
reduce the number of "Huh?" moments dramatically.
Also, I am opposed to make this a remote.<name>.trackIntoLocalBranches
variable.
IMHO a global flag should be enough, if you really need it at all. Your
code now makes sure that the defaults will be set only if there is a
proper fetch entry for the corresponding remote name, so it is safe.
People wanting to prevent that behaviour can use "--no-track", and those
who don't know about "--no-track" are probably better off _with_ DWIM
defaults for the new branch.
> >> @@ -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.
>
> If warn_ambiguous_ref == 0, dwim_ref is never going to answer anything >
> 1...
Right. Thank you!
Ciao,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html