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

[PATCH 7/8] fetch: fix inconsistent summary width for pruned and updated refs

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 15, 2023, 11:21 UTC
Message-ID
<f67f9640a853b605dd1bc4be25e9988c4f059684.1678878623.git.ps@pks.im>
In-Reply-To
<cover.1678878623.git.ps@pks.im>

When printing references that are updated during a fetch, we also print a short summary of what is happening. This summary may either be a word like "[up to date]" or "[rejected]", or the abbreviated old and new hash on a reference update.

As the abbreviated hashes may have different lengths in order to be unique we thus need to precompute the width of the summary's column by iterating through all the objects. This is done in two locations: once to compute the width for references that are to be pruned, and once for all the other references. Consequentially, it can happen that the width as calculated for these sets of references is different.

This isn't really a huge issue, but it is a bit confusing when reading through the code. Furthermore, the current way of passing through the summary width is rather verbose.

Refactor the code to compute the summary width once when initializing the display state. Like this, the width will be the same and we have to pass less variables around.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fetch.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4e18c3902d..31724e9aaf 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -49,6 +49,7 @@ enum {
 
 struct display_state {
 	int refcol_width;
+	int summary_width;
 	int compact_format;
 
 	char *url;
@@ -830,6 +831,8 @@ static void display_state_init(struct display_state *display, struct ref *ref_ma
 		if (display->refcol_width < width)
 			display->refcol_width = width;
 	}
+
+	display->summary_width = transport_summary_width(ref_map);
 }
 
 static void display_state_release(struct display_state *display)
@@ -896,8 +899,7 @@ static void print_compact(struct display_state *display, struct strbuf *display_
 static void format_display(struct display_state *display,
 			   struct strbuf *display_buffer, char code,
 			   const char *summary, const char *error,
-			   const char *remote, const char *local,
-			   int summary_width)
+			   const char *remote, const char *local)
 {
 	int width;
 
@@ -909,7 +911,7 @@ static void format_display(struct display_state *display,
 		display->shown_url = 1;
 	}
 
-	width = (summary_width + strlen(summary) - gettext_width(summary));
+	width = (display->summary_width + strlen(summary) - gettext_width(summary));
 
 	strbuf_addf(display_buffer, " %c %-*s ", code, width, summary);
 	if (!display->compact_format)
@@ -925,7 +927,7 @@ static int update_local_ref(struct ref *ref,
 			    struct ref_transaction *transaction,
 			    struct display_state *display,
 			    const char *remote, const struct ref *remote_ref,
-			    struct strbuf *display_buffer, int summary_width)
+			    struct strbuf *display_buffer)
 {
 	struct commit *current = NULL, *updated;
 	int fast_forward = 0;
@@ -936,7 +938,7 @@ static int update_local_ref(struct ref *ref,
 	if (oideq(&ref->old_oid, &ref->new_oid)) {
 		if (verbosity > 0)
 			format_display(display, display_buffer, '=', _("[up to date]"), NULL,
-				       remote, ref->name, summary_width);
+				       remote, ref->name);
 		return 0;
 	}
 
@@ -949,7 +951,7 @@ static int update_local_ref(struct ref *ref,
 		 */
 		format_display(display, display_buffer, '!', _("[rejected]"),
 			       _("can't fetch into checked-out branch"),
-			       remote, ref->name, summary_width);
+			       remote, ref->name);
 		return 1;
 	}
 
@@ -960,12 +962,12 @@ static int update_local_ref(struct ref *ref,
 			r = s_update_ref("updating tag", ref, transaction, 0);
 			format_display(display, display_buffer, r ? '!' : 't', _("[tag update]"),
 				       r ? _("unable to update local ref") : NULL,
-				       remote, ref->name, summary_width);
+				       remote, ref->name);
 			return r;
 		} else {
 			format_display(display, display_buffer, '!', _("[rejected]"),
 				       _("would clobber existing tag"),
-				       remote, ref->name, summary_width);
+				       remote, ref->name);
 			return 1;
 		}
 	}
@@ -998,7 +1000,7 @@ static int update_local_ref(struct ref *ref,
 		r = s_update_ref(msg, ref, transaction, 0);
 		format_display(display, display_buffer, r ? '!' : '*', what,
 			       r ? _("unable to update local ref") : NULL,
-			       remote, ref->name, summary_width);
+			       remote, ref->name);
 		return r;
 	}
 
@@ -1020,7 +1022,7 @@ static int update_local_ref(struct ref *ref,
 		r = s_update_ref("fast-forward", ref, transaction, 1);
 		format_display(display, display_buffer, r ? '!' : ' ', quickref.buf,
 			       r ? _("unable to update local ref") : NULL,
-			       remote, ref->name, summary_width);
+			       remote, ref->name);
 		strbuf_release(&quickref);
 		return r;
 	} else if (force || ref->force) {
@@ -1032,12 +1034,12 @@ static int update_local_ref(struct ref *ref,
 		r = s_update_ref("forced-update", ref, transaction, 1);
 		format_display(display, display_buffer, r ? '!' : '+', quickref.buf,
 			       r ? _("unable to update local ref") : _("forced update"),
-			       remote, ref->name, summary_width);
+			       remote, ref->name);
 		strbuf_release(&quickref);
 		return r;
 	} else {
 		format_display(display, display_buffer, '!', _("[rejected]"), _("non-fast-forward"),
-			       remote, ref->name, summary_width);
+			       remote, ref->name);
 		return 1;
 	}
 }
@@ -1158,10 +1160,6 @@ static int store_updated_refs(struct display_state *display,
 	const char *what, *kind;
 	struct ref *rm;
 	int want_status;
-	int summary_width = 0;
-
-	if (verbosity >= 0)
-		summary_width = transport_summary_width(ref_map);
 
 	if (!connectivity_checked) {
 		struct check_connected_options opt = CHECK_CONNECTED_INIT;
@@ -1266,7 +1264,7 @@ static int store_updated_refs(struct display_state *display,
 			strbuf_reset(&note);
 			if (ref) {
 				rc |= update_local_ref(ref, transaction, display, what,
-						       rm, &note, summary_width);
+						       rm, &note);
 				free(ref);
 			} else if (write_fetch_head || dry_run) {
 				/*
@@ -1277,7 +1275,7 @@ static int store_updated_refs(struct display_state *display,
 				format_display(display, &note, '*',
 					       *kind ? kind : "branch", NULL,
 					       *what ? what : "HEAD",
-					       "FETCH_HEAD", summary_width);
+					       "FETCH_HEAD");
 			}
 			if (note.len)
 				fputs(note.buf, stderr);
@@ -1413,13 +1411,10 @@ static int prune_refs(struct display_state *display,
 	}
 
 	if (verbosity >= 0) {
-		int summary_width = transport_summary_width(stale_refs);
-
 		for (ref = stale_refs; ref; ref = ref->next) {
 			struct strbuf sb = STRBUF_INIT;
 			format_display(display, &sb, '-', _("[deleted]"), NULL,
-				       _("(none)"), ref->name,
-				       summary_width);
+				       _("(none)"), ref->name);
 			fputs(sb.buf, stderr);
 			strbuf_release(&sb);
 			warn_dangling_symref(stderr, dangling_msg, ref->name);

-- 
2.40.0
Previous: Junio C HamanoNext: Junio C Hamano
Message 18 of 39 in “fetch: refactor code that prints reference updates”
  1. 0/8 fetch: refactor code that prints reference updatesPatrick Steinhardt, Mar 15, 2023
  2. 1/8 fetch: rename `display` buffer to avoid name conflictPatrick Steinhardt, Mar 15, 2023
  3. 2/8 fetch: move reference width calculation into `display_state`Patrick Steinhardt, Mar 15, 2023
  4. Junio C HamanoMar 15, 2023
  5. Patrick SteinhardtMar 16, 2023
  6. Junio C HamanoMar 16, 2023
  7. Patrick SteinhardtMar 17, 2023
  8. Junio C HamanoMar 16, 2023
  9. 3/8 fetch: move output format into `display_state`Patrick Steinhardt, Mar 15, 2023
  10. 4/8 fetch: pass the full local reference name to `format_display`Patrick Steinhardt, Mar 15, 2023
  11. Junio C HamanoMar 15, 2023
  12. 5/8 fetch: deduplicate handling of per-reference formatPatrick Steinhardt, Mar 15, 2023
  13. Junio C HamanoMar 15, 2023
  14. Patrick SteinhardtMar 16, 2023
  15. Junio C HamanoMar 16, 2023
  16. Patrick SteinhardtMar 17, 2023
  17. Junio C HamanoMar 17, 2023
  18. 7/8 fetch: fix inconsistent summary width for pruned and updated refsPatrick Steinhardt, Mar 15, 2023
  19. Junio C HamanoMar 15, 2023
  20. Patrick SteinhardtMar 16, 2023
  21. Junio C HamanoMar 16, 2023
  22. Patrick SteinhardtMar 17, 2023
  23. 6/8 fetch: deduplicate logic to print remote URLPatrick Steinhardt, Mar 15, 2023
  24. Junio C HamanoMar 15, 2023
  25. Patrick SteinhardtMar 16, 2023
  26. 8/8 fetch: centralize printing of reference updatesPatrick Steinhardt, Mar 15, 2023
  27. Jonathan TanMar 17, 2023
  28. Patrick SteinhardtMar 20, 2023
  29. Patrick SteinhardtMar 20, 2023
  30. 0/6 fetch: refactor code that prints reference updatesPatrick Steinhardt, Mar 20, 2023
  31. 1/6 fetch: move reference width calculation into `display_state`Patrick Steinhardt, Mar 20, 2023
  32. 2/6 fetch: move output format into `display_state`Patrick Steinhardt, Mar 20, 2023
  33. 3/6 fetch: pass the full local reference name to `format_display`Patrick Steinhardt, Mar 20, 2023
  34. 4/6 fetch: centralize handling of per-reference formatPatrick Steinhardt, Mar 20, 2023
  35. 5/6 fetch: centralize logic to print remote URLPatrick Steinhardt, Mar 20, 2023
  36. 6/6 fetch: centralize printing of reference updatesPatrick Steinhardt, Mar 20, 2023
  37. Jonathan TanMar 20, 2023
  38. Patrick SteinhardtMar 22, 2023
  39. Junio C HamanoMar 29, 2023

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.