git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/5] interpret_branch_name: factor out upstream handling

From
Jeff King <peff@peff.net>
Date
Jan 15, 2014, 08:26 UTC
Message-ID
<20140115082632.GA19132@sigill.intra.peff.net>
In-Reply-To
<20140115082528.GA18974@sigill.intra.peff.net>

This function checks a few different @{}-constructs. The early part checks for and dispatches us to helpers for each construct, but the code for handling @{upstream} is inline.

Let's factor this out into its own function. This makes interpret_branch_name more readable, and will make it much simpler to further refactor the function in future patches.

While we're at it, let's also break apart the refactored code into a few helper functions. These will be useful if we eventually implement similar @{upstream}-like constructs.

Signed-off-by: Jeff King <peff@peff.net>
---
This is identical to the cleanup I posted recently for the @{publish}
topic, though I did tweak the commit message a bit to make more sense in
the context of this series.
 sha1_name.c | 83 ++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 52 insertions(+), 31 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index a5578f7..5db742b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1046,6 +1046,54 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu
 	return ret - used + len;
 }
 
+static void set_shortened_ref(struct strbuf *buf, const char *ref)
+{
+	char *s = shorten_unambiguous_ref(ref, 0);
+	strbuf_reset(buf);
+	strbuf_addstr(buf, s);
+	free(s);
+}
+
+static const char *get_upstream_branch(const char *branch_buf, int len)
+{
+	char *branch = xstrndup(branch_buf, len);
+	struct branch *upstream = branch_get(*branch ? branch : NULL);
+
+	/*
+	 * Upstream can be NULL only if branch refers to HEAD and HEAD
+	 * points to something different than a branch.
+	 */
+	if (!upstream)
+		die(_("HEAD does not point to a branch"));
+	if (!upstream->merge || !upstream->merge[0]->dst) {
+		if (!ref_exists(upstream->refname))
+			die(_("No such branch: '%s'"), branch);
+		if (!upstream->merge) {
+			die(_("No upstream configured for branch '%s'"),
+				upstream->name);
+		}
+		die(
+			_("Upstream branch '%s' not stored as a remote-tracking branch"),
+			upstream->merge[0]->src);
+	}
+	free(branch);
+
+	return upstream->merge[0]->dst;
+}
+
+static int interpret_upstream_mark(const char *name, int namelen,
+				   int at, struct strbuf *buf)
+{
+	int len;
+
+	len = upstream_mark(name + at, namelen - at);
+	if (!len)
+		return -1;
+
+	set_shortened_ref(buf, get_upstream_branch(name, at));
+	return len + at;
+}
+
 /*
  * This reads short-hand syntax that not only evaluates to a commit
  * object name, but also can act as if the end user spelled the name
@@ -1070,9 +1118,7 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu
 int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
 {
 	char *cp;
-	struct branch *upstream;
 	int len = interpret_nth_prior_checkout(name, buf);
-	int tmp_len;
 
 	if (!namelen)
 		namelen = strlen(name);
@@ -1094,36 +1140,11 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
 	if (len > 0)
 		return reinterpret(name, namelen, len, buf);
 
-	tmp_len = upstream_mark(cp, namelen - (cp - name));
-	if (!tmp_len)
-		return -1;
+	len = interpret_upstream_mark(name, namelen, cp - name, buf);
+	if (len > 0)
+		return len;
 
-	len = cp + tmp_len - name;
-	cp = xstrndup(name, cp - name);
-	upstream = branch_get(*cp ? cp : NULL);
-	/*
-	 * Upstream can be NULL only if cp refers to HEAD and HEAD
-	 * points to something different than a branch.
-	 */
-	if (!upstream)
-		die(_("HEAD does not point to a branch"));
-	if (!upstream->merge || !upstream->merge[0]->dst) {
-		if (!ref_exists(upstream->refname))
-			die(_("No such branch: '%s'"), cp);
-		if (!upstream->merge) {
-			die(_("No upstream configured for branch '%s'"),
-				upstream->name);
-		}
-		die(
-			_("Upstream branch '%s' not stored as a remote-tracking branch"),
-			upstream->merge[0]->src);
-	}
-	free(cp);
-	cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
-	strbuf_reset(buf);
-	strbuf_addstr(buf, cp);
-	free(cp);
-	return len;
+	return -1;
 }
 
 int strbuf_branchname(struct strbuf *sb, const char *name)
-- 
1.8.5.2.500.g8060133
Previous: Jeff KingNext: Jeff King
Message 7 of 12 in “BUG: check-ref-format and rev-parse can not handle branches with an @ in their name combined with @{u}”
  1. Keith DerrickJan 14, 2014
  2. Junio C HamanoJan 14, 2014
  3. Jeff KingJan 15, 2014
  4. Junio C HamanoJan 15, 2014
  5. Jeff KingJan 15, 2014
  6. 0/5 interpret_branch_name bug potpourriJeff King, Jan 15, 2014
  7. 1/5 interpret_branch_name: factor out upstream handlingJeff King, Jan 15, 2014
  8. 2/5 interpret_branch_name: rename "cp" variable to "at"Jeff King, Jan 15, 2014
  9. 3/5 interpret_branch_name: always respect "namelen" parameterJeff King, Jan 15, 2014
  10. 4/5 interpret_branch_name: avoid @{upstream} past colonJeff King, Jan 15, 2014
  11. 5/5 interpret_branch_name: find all possible @-marksJeff King, Jan 15, 2014
  12. Junio C HamanoJan 15, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.