Re: Re* [BUG] "git checkout -b" erronously thinks a branch already exists

From: Jeff King <peff@peff.net>
Date: Jun 6, 2011, 16:05 UTC
Message-ID: <20110606160538.GA11485@sigill.intra.peff.net>
In-Reply-To: <7v1uz7bkc9.fsf@alter.siamese.dyndns.org>

On Sun, Jun 05, 2011 at 10:27:34PM -0700, Junio C Hamano wrote:

Show 5 quoted lines

> Let's do this instead. I don't know what I was thinking when I wrote that
> inefficient "loop refs to see if there is that one" patch.
> 
> -- >8 --
> Subject: [PATCH] checkout -b <name>: correctly detect existing branch

Yeah, I did a double-take seeing your other patch and wondering why you weren't just using resolve_ref.

So this patch looks good to me.

Show 5 quoted lines

> +int ref_exists(char *refname)
> +{
> +	unsigned char sha1[20];
> +	return !!resolve_ref(refname, sha1, 1, NULL);
> +}

I was tempted to suggest that ref_exists could be used in lots of other places to make the code slightly more readable. But in many cases, the variable holding the dummy sha1 cannot go away (because it is used elsewhere in the function), which means manually figuring out whether or not the sha1 was actually a dummy or not[1]. So it's probably not worth the effort for such minor gain.

-Peff

[1] For example, the one in create_branch is OK to change, but the one in delete_branch is not.