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

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

+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