Re: [PATCH 2/3] builtin-remote: teach show to display remote HEAD

From: Jeff King <peff@peff.net>
Date: Feb 12, 2009, 00:26 UTC
Message-ID: <20090212002612.GC30231@coredump.intra.peff.net>
In-Reply-To: <1234332083-45147-3-git-send-email-jaysoffian@gmail.com>

On Wed, Feb 11, 2009 at 01:01:22AM -0500, Jay Soffian wrote:

Show 31 quoted lines

> +static char *get_head_name(const struct ref *ref)
> +{
> +	const struct ref *remote_head = NULL;
> +	const struct ref *remote_master = NULL;
> +	const struct ref *r;
> +	for (r = ref; r; r = r->next) {
> +		if (!strcmp(r->name, "HEAD"))
> +			remote_head = r;
> +		if (!strcmp(r->name, "refs/heads/master"))
> +			remote_master = r;
> +	}
> +
> +	/* If there's no HEAD value at all, never mind. */
> +	if (!remote_head)
> +		return NULL;
> +
> +	/* If refs/heads/master could be right, it is. */
> +	if (remote_master && !hashcmp(remote_master->old_sha1,
> +				      remote_head->old_sha1))
> +		return xstrdup(abbrev_branch(remote_master->name));
> +
> +	/* Look for another ref that points there */
> +	for (r = ref; r; r = r->next)
> +		if (r != remote_head &&
> +		    !hashcmp(r->old_sha1, remote_head->old_sha1) &&
> +		    !prefixcmp(r->name, "refs/heads/"))
> +			return xstrdup(abbrev_branch(r->name));
> +
> +	/* Nothing is the same */
> +	return NULL;
> +}

Yuck. Cut and paste from builtin-clone.c. It's not so much the number of lines here (although of course I don't like that, either) but that this function encompasses a heuristic for matching the HEAD. Which means it may change in the future, and I really don't want the clone behavior and the remote behavior to diverge.

So can we refactor it into a library function?

I see that the inputs and outputs aren't exactly the same in both cases, but I think you could do it like:

  struct ref *guess_head_ref(const struct ref *refs_with_head,
                             const struct ref *refs_that_might_match_head,
                             struct ref **remote_head_p);

and then just call:

  r = guess_head_ref(refs, refs, NULL);
  states->head_name = r ? xstrdup(abbrev_branch(r->name)) : NULL;

from git-remote, which at least keeps the changeable parts all contained.

-Peff