From: John Tapsell <johnflux@gmail.com>
Date: Thu, 12 Mar 2009 17:45:56 GMT
Subject: Re: git checkout -b origin/mybranch origin/mybranch
Message-ID: <43d8ce650903121045s74584d97o708d4f8fd2baf664@mail.gmail.com>
In-Reply-To: <20090312171420.GA2192@coredump.intra.peff.net>

2009/3/12 Jeff King <peff@peff.net>:
> On Thu, Mar 12, 2009 at 04:58:22PM +0000, John Tapsell wrote:
>
>> > -       if (resolve_ref(ref.buf, sha1, 1, NULL)) {
>> > +       if (dwim_ref(name, strlen(name), sha1, &junk)) {
>> > +               free(junk);
>>
>> Presumably 'junk' is the resolved name?  I wonder if it's worth
>> putting this info in the error message?
>
> Hey, I said it was sloppy, right? ;)
>
> Here's your suggestion, plus specifying which situation (existing branch
> or ambiguous ref) would occur. It would still need tests. But I'm
> curious to hear more opinions on this direction before cleaning it up
> much more (at the very least, it needs some tests).

I like :-)  Just minor comments:

> --- a/branch.c
> +++ b/branch.c
> @@ -133,6 +133,7 @@ void create_branch(const char *head,
>        unsigned char sha1[20];
>        char *real_ref, msg[PATH_MAX + 20];
>        struct strbuf ref = STRBUF_INIT;
> +       char *existing;

Don't suppose you could set this NULL.  Just in case dwim_ref doesn't
set &existing for whatever reason.

>        int forcing = 0;
>        int len;
>
> @@ -146,12 +147,18 @@ void create_branch(const char *head,
>        if (check_ref_format(ref.buf))
>                die("'%s' is not a valid branch name.", name);
>
> -       if (resolve_ref(ref.buf, sha1, 1, NULL)) {
> -               if (!force)
> -                       die("A branch named '%s' already exists.", name);
> +       if (dwim_ref(name, strlen(name), sha1, &existing)) {
> +               if (!force) {
> +                       if (!prefixcmp(existing, "refs/heads/"))
> +                               die("A branch named '%s' already exists.",
> +                                   name);
> +                       die("Creating '%s' would be ambiguous with"
> +                           " the existing %s", name, existing);

Maybe put single quotes around the second %s, for consistency with the first?

> +               }
>                else if (!is_bare_repository() && !strcmp(head, name))
>                        die("Cannot force update the current branch.");
>                forcing = 1;
> +               free(existing);
>        }
>
>        real_ref = NULL;
>