[PATCH v2 1/6] fetch: move reference width calculation into `display_state`

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 14542 bytes --]

In order to print references in proper columns we need to calculate the
width of the reference column before starting to print the references.
This is done with the help of a global variable `refcol_width`.

Refactor the code to instead use a new structure `display_state` that
contains the computed width and plumb it through the stack as required.
This is only the first step towards de-globalizing the state required to
print references.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fetch.c | 111 ++++++++++++++++++++++++++++--------------------
 1 file changed, 65 insertions(+), 46 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index a09606b472..391959ad3d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -47,6 +47,10 @@ enum {
 	TAGS_SET = 2
 };
 
+struct display_state {
+	int refcol_width;
+};
+
 static int fetch_prune_config = -1; /* unspecified */
 static int fetch_show_forced_updates = 1;
 static uint64_t forced_updates_ms = 0;
@@ -741,16 +745,15 @@ static int s_update_ref(const char *action,
 	return ret;
 }
 
-static int refcol_width = 10;
 static int compact_format;
 
-static void adjust_refcol_width(const struct ref *ref)
+static int refcol_width(const struct ref *ref)
 {
 	int max, rlen, llen, len;
 
 	/* uptodate lines are only shown on high verbosity level */
 	if (verbosity <= 0 && oideq(&ref->peer_ref->old_oid, &ref->old_oid))
-		return;
+		return 0;
 
 	max    = term_columns();
 	rlen   = utf8_strwidth(prettify_refname(ref->name));
@@ -769,22 +772,18 @@ static void adjust_refcol_width(const struct ref *ref)
 	}
 	len = 21 /* flag and summary */ + rlen + 4 /* -> */ + llen;
 	if (len >= max)
-		return;
+		return 0;
 
-	/*
-	 * Not precise calculation for compact mode because '*' can
-	 * appear on the left hand side of '->' and shrink the column
-	 * back.
-	 */
-	if (refcol_width < rlen)
-		refcol_width = rlen;
+	return rlen;
 }
 
-static void prepare_format_display(struct ref *ref_map)
+static void display_state_init(struct display_state *display_state, struct ref *ref_map)
 {
 	struct ref *rm;
 	const char *format = "full";
 
+	memset(display_state, 0, sizeof(*display_state));
+
 	if (verbosity < 0)
 		return;
 
@@ -797,20 +796,32 @@ static void prepare_format_display(struct ref *ref_map)
 		die(_("invalid value for '%s': '%s'"),
 		    "fetch.output", format);
 
+	display_state->refcol_width = 10;
 	for (rm = ref_map; rm; rm = rm->next) {
+		int width;
+
 		if (rm->status == REF_STATUS_REJECT_SHALLOW ||
 		    !rm->peer_ref ||
 		    !strcmp(rm->name, "HEAD"))
 			continue;
 
-		adjust_refcol_width(rm);
+		width = refcol_width(rm);
+
+		/*
+		 * Not precise calculation for compact mode because '*' can
+		 * appear on the left hand side of '->' and shrink the column
+		 * back.
+		 */
+		if (display_state->refcol_width < width)
+			display_state->refcol_width = width;
 	}
 }
 
-static void print_remote_to_local(struct strbuf *display,
+static void print_remote_to_local(struct display_state *display_state,
+				  struct strbuf *display_buffer,
 				  const char *remote, const char *local)
 {
-	strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local);
+	strbuf_addf(display_buffer, "%-*s -> %s", display_state->refcol_width, remote, local);
 }
 
 static int find_and_replace(struct strbuf *haystack,
@@ -840,14 +851,14 @@ static int find_and_replace(struct strbuf *haystack,
 	return 1;
 }
 
-static void print_compact(struct strbuf *display,
+static void print_compact(struct display_state *display_state, struct strbuf *display_buffer,
 			  const char *remote, const char *local)
 {
 	struct strbuf r = STRBUF_INIT;
 	struct strbuf l = STRBUF_INIT;
 
 	if (!strcmp(remote, local)) {
-		strbuf_addf(display, "%-*s -> *", refcol_width, remote);
+		strbuf_addf(display_buffer, "%-*s -> *", display_state->refcol_width, remote);
 		return;
 	}
 
@@ -856,13 +867,14 @@ static void print_compact(struct strbuf *display,
 
 	if (!find_and_replace(&r, local, "*"))
 		find_and_replace(&l, remote, "*");
-	print_remote_to_local(display, r.buf, l.buf);
+	print_remote_to_local(display_state, display_buffer, r.buf, l.buf);
 
 	strbuf_release(&r);
 	strbuf_release(&l);
 }
 
-static void format_display(struct strbuf *display, char code,
+static void format_display(struct display_state *display_state,
+			   struct strbuf *display_buffer, char code,
 			   const char *summary, const char *error,
 			   const char *remote, const char *local,
 			   int summary_width)
@@ -874,17 +886,18 @@ static void format_display(struct strbuf *display, char code,
 
 	width = (summary_width + strlen(summary) - gettext_width(summary));
 
-	strbuf_addf(display, "%c %-*s ", code, width, summary);
+	strbuf_addf(display_buffer, "%c %-*s ", code, width, summary);
 	if (!compact_format)
-		print_remote_to_local(display, remote, local);
+		print_remote_to_local(display_state, display_buffer, remote, local);
 	else
-		print_compact(display, remote, local);
+		print_compact(display_state, display_buffer, remote, local);
 	if (error)
-		strbuf_addf(display, "  (%s)", error);
+		strbuf_addf(display_buffer, "  (%s)", error);
 }
 
 static int update_local_ref(struct ref *ref,
 			    struct ref_transaction *transaction,
+			    struct display_state *display_state,
 			    const char *remote, const struct ref *remote_ref,
 			    struct strbuf *display, int summary_width)
 {
@@ -897,7 +910,7 @@ static int update_local_ref(struct ref *ref,
 
 	if (oideq(&ref->old_oid, &ref->new_oid)) {
 		if (verbosity > 0)
-			format_display(display, '=', _("[up to date]"), NULL,
+			format_display(display_state, display, '=', _("[up to date]"), NULL,
 				       remote, pretty_ref, summary_width);
 		return 0;
 	}
@@ -909,7 +922,7 @@ static int update_local_ref(struct ref *ref,
 		 * If this is the head, and it's not okay to update
 		 * the head, and the old value of the head isn't empty...
 		 */
-		format_display(display, '!', _("[rejected]"),
+		format_display(display_state, display, '!', _("[rejected]"),
 			       _("can't fetch into checked-out branch"),
 			       remote, pretty_ref, summary_width);
 		return 1;
@@ -920,12 +933,13 @@ static int update_local_ref(struct ref *ref,
 		if (force || ref->force) {
 			int r;
 			r = s_update_ref("updating tag", ref, transaction, 0);
-			format_display(display, r ? '!' : 't', _("[tag update]"),
+			format_display(display_state, display, r ? '!' : 't', _("[tag update]"),
 				       r ? _("unable to update local ref") : NULL,
 				       remote, pretty_ref, summary_width);
 			return r;
 		} else {
-			format_display(display, '!', _("[rejected]"), _("would clobber existing tag"),
+			format_display(display_state, display, '!', _("[rejected]"),
+				       _("would clobber existing tag"),
 				       remote, pretty_ref, summary_width);
 			return 1;
 		}
@@ -957,7 +971,7 @@ static int update_local_ref(struct ref *ref,
 		}
 
 		r = s_update_ref(msg, ref, transaction, 0);
-		format_display(display, r ? '!' : '*', what,
+		format_display(display_state, display, r ? '!' : '*', what,
 			       r ? _("unable to update local ref") : NULL,
 			       remote, pretty_ref, summary_width);
 		return r;
@@ -979,7 +993,7 @@ static int update_local_ref(struct ref *ref,
 		strbuf_addstr(&quickref, "..");
 		strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);
 		r = s_update_ref("fast-forward", ref, transaction, 1);
-		format_display(display, r ? '!' : ' ', quickref.buf,
+		format_display(display_state, display, r ? '!' : ' ', quickref.buf,
 			       r ? _("unable to update local ref") : NULL,
 			       remote, pretty_ref, summary_width);
 		strbuf_release(&quickref);
@@ -991,13 +1005,13 @@ static int update_local_ref(struct ref *ref,
 		strbuf_addstr(&quickref, "...");
 		strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);
 		r = s_update_ref("forced-update", ref, transaction, 1);
-		format_display(display, r ? '!' : '+', quickref.buf,
+		format_display(display_state, display, r ? '!' : '+', quickref.buf,
 			       r ? _("unable to update local ref") : _("forced update"),
 			       remote, pretty_ref, summary_width);
 		strbuf_release(&quickref);
 		return r;
 	} else {
-		format_display(display, '!', _("[rejected]"), _("non-fast-forward"),
+		format_display(display_state, display, '!', _("[rejected]"), _("non-fast-forward"),
 			       remote, pretty_ref, summary_width);
 		return 1;
 	}
@@ -1108,7 +1122,8 @@ N_("it took %.2f seconds to check forced updates; you can use\n"
    "'--no-show-forced-updates' or run 'git config fetch.showForcedUpdates false'\n"
    "to avoid this check\n");
 
-static int store_updated_refs(const char *raw_url, const char *remote_name,
+static int store_updated_refs(struct display_state *display_state,
+			      const char *raw_url, const char *remote_name,
 			      int connectivity_checked,
 			      struct ref_transaction *transaction, struct ref *ref_map,
 			      struct fetch_head *fetch_head)
@@ -1139,8 +1154,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		}
 	}
 
-	prepare_format_display(ref_map);
-
 	/*
 	 * We do a pass for each fetch_head_status type in their enum order, so
 	 * merged entries are written before not-for-merge. That lets readers
@@ -1240,7 +1253,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 
 			strbuf_reset(&note);
 			if (ref) {
-				rc |= update_local_ref(ref, transaction, what,
+				rc |= update_local_ref(ref, transaction, display_state, what,
 						       rm, &note, summary_width);
 				free(ref);
 			} else if (write_fetch_head || dry_run) {
@@ -1249,7 +1262,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				 * would be written to FETCH_HEAD, if --dry-run
 				 * is set).
 				 */
-				format_display(&note, '*',
+				format_display(display_state, &note, '*',
 					       *kind ? kind : "branch", NULL,
 					       *what ? what : "HEAD",
 					       "FETCH_HEAD", summary_width);
@@ -1328,7 +1341,8 @@ static int check_exist_and_connected(struct ref *ref_map)
 	return check_connected(iterate_ref_map, &rm, &opt);
 }
 
-static int fetch_and_consume_refs(struct transport *transport,
+static int fetch_and_consume_refs(struct display_state *display_state,
+				  struct transport *transport,
 				  struct ref_transaction *transaction,
 				  struct ref *ref_map,
 				  struct fetch_head *fetch_head)
@@ -1352,7 +1366,7 @@ static int fetch_and_consume_refs(struct transport *transport,
 	}
 
 	trace2_region_enter("fetch", "consume_refs", the_repository);
-	ret = store_updated_refs(transport->url, transport->remote->name,
+	ret = store_updated_refs(display_state, transport->url, transport->remote->name,
 				 connectivity_checked, transaction, ref_map,
 				 fetch_head);
 	trace2_region_leave("fetch", "consume_refs", the_repository);
@@ -1362,7 +1376,8 @@ static int fetch_and_consume_refs(struct transport *transport,
 	return ret;
 }
 
-static int prune_refs(struct refspec *rs,
+static int prune_refs(struct display_state *display_state,
+		      struct refspec *rs,
 		      struct ref_transaction *transaction,
 		      struct ref *ref_map,
 		      const char *raw_url)
@@ -1416,7 +1431,7 @@ static int prune_refs(struct refspec *rs,
 				fprintf(stderr, _("From %.*s\n"), url_len, url);
 				shown_url = 1;
 			}
-			format_display(&sb, '-', _("[deleted]"), NULL,
+			format_display(display_state, &sb, '-', _("[deleted]"), NULL,
 				       _("(none)"), prettify_refname(ref->name),
 				       summary_width);
 			fprintf(stderr, " %s\n",sb.buf);
@@ -1542,7 +1557,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
 	return transport;
 }
 
-static int backfill_tags(struct transport *transport,
+static int backfill_tags(struct display_state *display_state,
+			 struct transport *transport,
 			 struct ref_transaction *transaction,
 			 struct ref *ref_map,
 			 struct fetch_head *fetch_head)
@@ -1566,7 +1582,7 @@ static int backfill_tags(struct transport *transport,
 	transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
 	transport_set_option(transport, TRANS_OPT_DEPTH, "0");
 	transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
-	retcode = fetch_and_consume_refs(transport, transaction, ref_map, fetch_head);
+	retcode = fetch_and_consume_refs(display_state, transport, transaction, ref_map, fetch_head);
 
 	if (gsecondary) {
 		transport_disconnect(gsecondary);
@@ -1581,6 +1597,7 @@ static int do_fetch(struct transport *transport,
 {
 	struct ref_transaction *transaction = NULL;
 	struct ref *ref_map = NULL;
+	struct display_state display_state;
 	int autotags = (transport->remote->fetch_tags == 1);
 	int retcode = 0;
 	const struct ref *remote_refs;
@@ -1662,6 +1679,8 @@ static int do_fetch(struct transport *transport,
 	if (retcode)
 		goto cleanup;
 
+	display_state_init(&display_state, ref_map);
+
 	if (atomic_fetch) {
 		transaction = ref_transaction_begin(&err);
 		if (!transaction) {
@@ -1679,9 +1698,9 @@ static int do_fetch(struct transport *transport,
 		 * don't care whether --tags was specified.
 		 */
 		if (rs->nr) {
-			retcode = prune_refs(rs, transaction, ref_map, transport->url);
+			retcode = prune_refs(&display_state, rs, transaction, ref_map, transport->url);
 		} else {
-			retcode = prune_refs(&transport->remote->fetch,
+			retcode = prune_refs(&display_state, &transport->remote->fetch,
 					     transaction, ref_map,
 					     transport->url);
 		}
@@ -1689,7 +1708,7 @@ static int do_fetch(struct transport *transport,
 			retcode = 1;
 	}
 
-	if (fetch_and_consume_refs(transport, transaction, ref_map, &fetch_head)) {
+	if (fetch_and_consume_refs(&display_state, transport, transaction, ref_map, &fetch_head)) {
 		retcode = 1;
 		goto cleanup;
 	}
@@ -1711,7 +1730,7 @@ static int do_fetch(struct transport *transport,
 			 * when `--atomic` is passed: in that case we'll abort
 			 * the transaction and don't commit anything.
 			 */
-			if (backfill_tags(transport, transaction, tags_ref_map,
+			if (backfill_tags(&display_state, transport, transaction, tags_ref_map,
 					  &fetch_head))
 				retcode = 1;
 		}
-- 
2.40.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]