diff mbox series

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

Message ID ce2c4b61aee26461b4edb934ceccf57daa62b056.1679315383.git.ps@pks.im (mailing list archive)
State Accepted
Commit ce9636d645714974e5f7e1e9abc65fa57186b147
Headers show
Series fetch: refactor code that prints reference updates | expand

Commit Message

Patrick Steinhardt March 20, 2023, 12:35 p.m. UTC 
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 mbox series

Patch

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;
 		}