Re: [PATCH v4 15/16] branch: use ref-filter printing APIs

[Jeff King <peff@peff.net>]

On Thu, Apr 14, 2016 at 04:05:30PM -0400, Jeff King wrote:

> It looks like that's a little tricky for %(upstream) and %(push), which
> have extra tracking options, but it's pretty trivial for %(symref):
> [...]
> I suspect it could work for the remote_ref_atom_parser, too, if you did
> something like:

So here that is (handling both %(symref) and %(upstream), so replacing
what I sent a few minutes ago). It's only lightly tested by me, so there
may be more corner cases to think about. I kept things where they were
to create a minimal diff, but if it gets squashed in, it might be worth
re-ordering a little to avoid the need for forward declarations.

> I don't know if that would create weird corner cases with RR_SHORTEN
> and RR_TRACK, though, which are currently in the same enum (and thus
> cannot be used at the same time). But it's not like we parse
> "%(upstream:short:track)" anyway, I don't think. For each "%()"
> placeholder you have to choose one or the other: showing the tracking
> info, or showing the refname (possibly with modifiers). So ":track"
> isn't so much a modifier as "upstream:track" is this totally other
> thing.

So actually, we _do_ accept "%(upstream:short,track)" with your patch,
which is somewhat nonsensical. It just ignores "short" and takes
whatever option came last. Which is reasonable, though flagging an error
would also be reasonable (and I think is what existing git does). I
don't think it matters much either way.

---
diff --git a/ref-filter.c b/ref-filter.c
index 3435df1..951c57e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -50,6 +50,11 @@ struct if_then_else {
 		condition_satisfied : 1;
 };
 
+struct refname_atom {
+	enum { R_BASE, R_DIR, R_NORMAL, R_SHORT, R_STRIP } option;
+	unsigned int strip;
+};
+
 /*
  * An atom is a valid field atom listed below, possibly prefixed with
  * a "*" to denote deref_tag().
@@ -67,7 +72,8 @@ static struct used_atom {
 		char color[COLOR_MAXLEN];
 		struct align align;
 		struct {
-			enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT } option;
+			enum { RR_REF, RR_TRACK, RR_TRACKSHORT } option;
+			struct refname_atom refname;
 			unsigned int nobracket: 1;
 		} remote_ref;
 		struct {
@@ -82,16 +88,14 @@ static struct used_atom {
 			enum { O_FULL, O_LENGTH, O_SHORT } option;
 			unsigned int length;
 		} objectname;
-		enum { S_FULL, S_SHORT } symref;
-		struct {
-			enum { R_BASE, R_DIR, R_NORMAL, R_SHORT, R_STRIP } option;
-			unsigned int strip;
-		} refname;
+		struct refname_atom refname;
 	} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
 static int need_color_reset_at_eol;
 
+static const char *show_ref(struct refname_atom *atom, const char *refname);
+
 static void color_atom_parser(struct used_atom *atom, const char *color_value)
 {
 	if (!color_value)
@@ -100,13 +104,34 @@ static void color_atom_parser(struct used_atom *atom, const char *color_value)
 		die(_("unrecognized color: %%(color:%s)"), color_value);
 }
 
+static void refname_atom_parser_internal(struct refname_atom *atom,
+					 const char *arg, const char *name)
+{
+	if (!arg)
+		atom->option = R_NORMAL;
+	else if (!strcmp(arg, "short"))
+		atom->option = R_SHORT;
+	else if (skip_prefix(arg, "strip=", &arg)) {
+		atom->option = R_STRIP;
+		if (strtoul_ui(arg, 10, &atom->strip) || atom->strip <= 0)
+			die(_("positive value expected refname:strip=%s"), arg);
+	} else if (!strcmp(arg, "dir"))
+		atom->option = R_DIR;
+	else if (!strcmp(arg, "base"))
+		atom->option = R_BASE;
+	else
+		die(_("unrecognized %%(%s) argument: %s"), name, arg);
+}
+
 static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 {
 	struct string_list params = STRING_LIST_INIT_DUP;
 	int i;
 
 	if (!arg) {
-		atom->u.remote_ref.option = RR_NORMAL;
+		atom->u.remote_ref.option = RR_REF;
+		refname_atom_parser_internal(&atom->u.remote_ref.refname,
+					     arg, atom->name);
 		return;
 	}
 
@@ -116,16 +141,17 @@ static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 	for (i = 0; i < params.nr; i++) {
 		const char *s = params.items[i].string;
 
-		if (!strcmp(s, "short"))
-			atom->u.remote_ref.option = RR_SHORTEN;
-		else if (!strcmp(s, "track"))
+		if (!strcmp(s, "track"))
 			atom->u.remote_ref.option = RR_TRACK;
 		else if (!strcmp(s, "trackshort"))
 			atom->u.remote_ref.option = RR_TRACKSHORT;
 		else if (!strcmp(s, "nobracket"))
 			atom->u.remote_ref.nobracket = 1;
-		else
-			die(_("unrecognized format: %%(%s)"), atom->name);
+		else {
+			atom->u.remote_ref.option = RR_REF;
+			refname_atom_parser_internal(&atom->u.remote_ref.refname,
+						     s, atom->name);
+		}
 	}
 
 	string_list_clear(&params, 0);
@@ -244,31 +270,12 @@ static void if_atom_parser(struct used_atom *atom, const char *arg)
 
 static void symref_atom_parser(struct used_atom *atom, const char *arg)
 {
-	if (!arg)
-		atom->u.symref = S_FULL;
-	else if (!strcmp(arg, "short"))
-		atom->u.symref = S_SHORT;
-	else
-		die(_("unrecognized %%(symref) argument: %s"), arg);
+	return refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
 }
 
 static void refname_atom_parser(struct used_atom *atom, const char *arg)
 {
-	if (!arg)
-		atom->u.refname.option = R_NORMAL;
-	else if (!strcmp(arg, "short"))
-		atom->u.refname.option = R_SHORT;
-	else if (skip_prefix(arg, "strip=", &arg)) {
-		atom->u.contents.option = R_STRIP;
-		if (strtoul_ui(arg, 10, &atom->u.refname.strip) ||
-			atom->u.refname.strip <= 0)
-			die(_("positive value expected refname:strip=%s"), arg);
-	} else if (!strcmp(arg, "dir"))
-		atom->u.contents.option = R_DIR;
-	else if (!strcmp(arg, "base"))
-		atom->u.contents.option = R_BASE;
-	else
-		die(_("unrecognized %%(refname) argument: %s"), arg);
+	return refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
 }
 
 static struct {
@@ -1112,8 +1119,8 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 				    struct branch *branch, const char **s)
 {
 	int num_ours, num_theirs;
-	if (atom->u.remote_ref.option == RR_SHORTEN)
-		*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
+	if (atom->u.remote_ref.option == RR_REF)
+		*s = show_ref(&atom->u.remote_ref.refname, refname);
 	else if (atom->u.remote_ref.option == RR_TRACK) {
 		if (stat_tracking_info(branch, &num_ours,
 				       &num_theirs, NULL)) {
@@ -1145,8 +1152,8 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 			*s = ">";
 		else
 			*s = "<>";
-	} else /* RR_NORMAL */
-		*s = refname;
+	} else
+		die("BUG: unhandled RR_* enum");
 }
 
 char *get_head_description(void)
@@ -1184,41 +1191,43 @@ static const char *get_symref(struct used_atom *atom, struct ref_array_item *ref
 {
 	if (!ref->symref)
 		return "";
-	else if (atom->u.symref == S_SHORT)
-		return shorten_unambiguous_ref(ref->symref,
-					       warn_ambiguous_refs);
 	else
-		return ref->symref;
+		return show_ref(&atom->u.refname, ref->symref);
 }
 
 static const char *get_refname(struct used_atom *atom, struct ref_array_item *ref)
 {
 	if (ref->kind & FILTER_REFS_DETACHED_HEAD)
 		return get_head_description();
-	if (atom->u.refname.option == R_SHORT)
-		return shorten_unambiguous_ref(ref->refname, warn_ambiguous_refs);
-	else if (atom->u.refname.option == R_STRIP)
-		return strip_ref_components(ref->refname, atom->u.refname.strip);
-	else if (atom->u.refname.option == R_BASE) {
+	return show_ref(&atom->u.refname, ref->refname);
+}
+
+static const char *show_ref(struct refname_atom *atom, const char *refname)
+{
+	if (atom->option == R_SHORT)
+		return shorten_unambiguous_ref(refname, warn_ambiguous_refs);
+	else if (atom->option == R_STRIP)
+		return strip_ref_components(refname, atom->strip);
+	else if (atom->option == R_BASE) {
 		const char *sp, *ep;
 
-		if (skip_prefix(ref->refname, "refs/", &sp)) {
+		if (skip_prefix(refname, "refs/", &sp)) {
 			ep = strchr(sp, '/');
 			if (!ep)
 				return "";
 			return xstrndup(sp, ep - sp);
 		}
 		return "";
-	} else if (atom->u.refname.option == R_DIR) {
+	} else if (atom->option == R_DIR) {
 		const char *sp, *ep;
 
-		sp = ref->refname;
+		sp = refname;
 		ep = strrchr(sp, '/');
 		if (!ep)
 			return "";
 		return xstrndup(sp, ep - sp);
 	} else
-		return ref->refname;
+		return refname;
 }
 
 /*