All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Jonathan Tan <jonathantanmy@google.com>
Subject: [PATCH v2 6/6] fetch: centralize printing of reference updates
Date: Mon, 20 Mar 2023 13:35:40 +0100	[thread overview]
Message-ID: <fe7e2e85eb37cd4068b5160721663c21a16a8138.1679315383.git.ps@pks.im> (raw)
In-Reply-To: <cover.1679315383.git.ps@pks.im>

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

In order to print updated references during a fetch, the two different
call sites that do this will first call `format_display()` followed by a
call to `fputs()`. This is needlessly roundabout now that we have the
`display_state` structure that encapsulates all of the printing logic
for references.

Move displaying the reference updates into `format_display()` and rename
it to `display_ref_update()` to better match its new purpose, which
finalizes the conversion to make both the formatting and printing logic
of reference updates self-contained. This will make it easier to add new
output formats and printing to a different file descriptor than stderr.

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

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1e3599cb74..c202c18fb4 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -48,6 +48,8 @@ enum {
 };
 
 struct display_state {
+	struct strbuf buf;
+
 	int refcol_width;
 	int compact_format;
 
@@ -787,6 +789,8 @@ static void display_state_init(struct display_state *display_state, struct ref *
 
 	memset(display_state, 0, sizeof(*display_state));
 
+	strbuf_init(&display_state->buf, 0);
+
 	if (raw_url)
 		display_state->url = transport_anonymize_url(raw_url);
 	else
@@ -834,14 +838,15 @@ static void display_state_init(struct display_state *display_state, struct ref *
 
 static void display_state_release(struct display_state *display_state)
 {
+	strbuf_release(&display_state->buf);
 	free(display_state->url);
 }
 
 static void print_remote_to_local(struct display_state *display_state,
-				  struct strbuf *display_buffer,
 				  const char *remote, const char *local)
 {
-	strbuf_addf(display_buffer, "%-*s -> %s", display_state->refcol_width, remote, local);
+	strbuf_addf(&display_state->buf, "%-*s -> %s",
+		    display_state->refcol_width, remote, local);
 }
 
 static int find_and_replace(struct strbuf *haystack,
@@ -871,14 +876,14 @@ static int find_and_replace(struct strbuf *haystack,
 	return 1;
 }
 
-static void print_compact(struct display_state *display_state, struct strbuf *display_buffer,
+static void print_compact(struct display_state *display_state,
 			  const char *remote, const char *local)
 {
 	struct strbuf r = STRBUF_INIT;
 	struct strbuf l = STRBUF_INIT;
 
 	if (!strcmp(remote, local)) {
-		strbuf_addf(display_buffer, "%-*s -> *", display_state->refcol_width, remote);
+		strbuf_addf(&display_state->buf, "%-*s -> *", display_state->refcol_width, remote);
 		return;
 	}
 
@@ -887,46 +892,49 @@ static void print_compact(struct display_state *display_state, struct strbuf *di
 
 	if (!find_and_replace(&r, local, "*"))
 		find_and_replace(&l, remote, "*");
-	print_remote_to_local(display_state, display_buffer, r.buf, l.buf);
+	print_remote_to_local(display_state, r.buf, l.buf);
 
 	strbuf_release(&r);
 	strbuf_release(&l);
 }
 
-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)
+static void display_ref_update(struct display_state *display_state, char code,
+			       const char *summary, const char *error,
+			       const char *remote, const char *local,
+			       int summary_width)
 {
 	int width;
 
 	if (verbosity < 0)
 		return;
 
+	strbuf_reset(&display_state->buf);
+
 	if (!display_state->shown_url) {
-		strbuf_addf(display_buffer, _("From %.*s\n"),
+		strbuf_addf(&display_state->buf, _("From %.*s\n"),
 			    display_state->url_len, display_state->url);
 		display_state->shown_url = 1;
 	}
 
 	width = (summary_width + strlen(summary) - gettext_width(summary));
 
-	strbuf_addf(display_buffer, " %c %-*s ", code, width, summary);
+	strbuf_addf(&display_state->buf, " %c %-*s ", code, width, summary);
 	if (!display_state->compact_format)
-		print_remote_to_local(display_state, display_buffer, remote, prettify_refname(local));
+		print_remote_to_local(display_state, remote, prettify_refname(local));
 	else
-		print_compact(display_state, display_buffer, remote, prettify_refname(local));
+		print_compact(display_state, remote, prettify_refname(local));
 	if (error)
-		strbuf_addf(display_buffer, "  (%s)", error);
-	strbuf_addch(display_buffer, '\n');
+		strbuf_addf(&display_state->buf, "  (%s)", error);
+	strbuf_addch(&display_state->buf, '\n');
+
+	fputs(display_state->buf.buf, stderr);
 }
 
 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)
+			    int summary_width)
 {
 	struct commit *current = NULL, *updated;
 	int fast_forward = 0;
@@ -936,8 +944,8 @@ static int update_local_ref(struct ref *ref,
 
 	if (oideq(&ref->old_oid, &ref->new_oid)) {
 		if (verbosity > 0)
-			format_display(display_state, display, '=', _("[up to date]"), NULL,
-				       remote, ref->name, summary_width);
+			display_ref_update(display_state, '=', _("[up to date]"), NULL,
+					   remote, ref->name, summary_width);
 		return 0;
 	}
 
@@ -948,9 +956,9 @@ 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_state, display, '!', _("[rejected]"),
-			       _("can't fetch into checked-out branch"),
-			       remote, ref->name, summary_width);
+		display_ref_update(display_state, '!', _("[rejected]"),
+				   _("can't fetch into checked-out branch"),
+				   remote, ref->name, summary_width);
 		return 1;
 	}
 
@@ -959,14 +967,14 @@ 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_state, display, r ? '!' : 't', _("[tag update]"),
-				       r ? _("unable to update local ref") : NULL,
-				       remote, ref->name, summary_width);
+			display_ref_update(display_state, r ? '!' : 't', _("[tag update]"),
+					   r ? _("unable to update local ref") : NULL,
+					   remote, ref->name, summary_width);
 			return r;
 		} else {
-			format_display(display_state, display, '!', _("[rejected]"),
-				       _("would clobber existing tag"),
-				       remote, ref->name, summary_width);
+			display_ref_update(display_state, '!', _("[rejected]"),
+					   _("would clobber existing tag"),
+					   remote, ref->name, summary_width);
 			return 1;
 		}
 	}
@@ -997,9 +1005,9 @@ static int update_local_ref(struct ref *ref,
 		}
 
 		r = s_update_ref(msg, ref, transaction, 0);
-		format_display(display_state, display, r ? '!' : '*', what,
-			       r ? _("unable to update local ref") : NULL,
-			       remote, ref->name, summary_width);
+		display_ref_update(display_state, r ? '!' : '*', what,
+				   r ? _("unable to update local ref") : NULL,
+				   remote, ref->name, summary_width);
 		return r;
 	}
 
@@ -1019,9 +1027,9 @@ 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_state, display, r ? '!' : ' ', quickref.buf,
-			       r ? _("unable to update local ref") : NULL,
-			       remote, ref->name, summary_width);
+		display_ref_update(display_state, r ? '!' : ' ', quickref.buf,
+				   r ? _("unable to update local ref") : NULL,
+				   remote, ref->name, summary_width);
 		strbuf_release(&quickref);
 		return r;
 	} else if (force || ref->force) {
@@ -1031,14 +1039,14 @@ 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_state, display, r ? '!' : '+', quickref.buf,
-			       r ? _("unable to update local ref") : _("forced update"),
-			       remote, ref->name, summary_width);
+		display_ref_update(display_state, r ? '!' : '+', quickref.buf,
+				   r ? _("unable to update local ref") : _("forced update"),
+				   remote, ref->name, summary_width);
 		strbuf_release(&quickref);
 		return r;
 	} else {
-		format_display(display_state, display, '!', _("[rejected]"), _("non-fast-forward"),
-			       remote, ref->name, summary_width);
+		display_ref_update(display_state, '!', _("[rejected]"), _("non-fast-forward"),
+				   remote, ref->name, summary_width);
 		return 1;
 	}
 }
@@ -1266,10 +1274,9 @@ static int store_updated_refs(struct display_state *display_state,
 					  note.buf, display_state->url,
 					  display_state->url_len);
 
-			strbuf_reset(&note);
 			if (ref) {
 				rc |= update_local_ref(ref, transaction, display_state, what,
-						       rm, &note, summary_width);
+						       rm, summary_width);
 				free(ref);
 			} else if (write_fetch_head || dry_run) {
 				/*
@@ -1277,13 +1284,11 @@ static int store_updated_refs(struct display_state *display_state,
 				 * would be written to FETCH_HEAD, if --dry-run
 				 * is set).
 				 */
-				format_display(display_state, &note, '*',
-					       *kind ? kind : "branch", NULL,
-					       *what ? what : "HEAD",
-					       "FETCH_HEAD", summary_width);
+				display_ref_update(display_state, '*',
+						   *kind ? kind : "branch", NULL,
+						   *what ? what : "HEAD",
+						   "FETCH_HEAD", summary_width);
 			}
-			if (note.len)
-				fputs(note.buf, stderr);
 		}
 	}
 
@@ -1419,12 +1424,9 @@ static int prune_refs(struct display_state *display_state,
 		int summary_width = transport_summary_width(stale_refs);
 
 		for (ref = stale_refs; ref; ref = ref->next) {
-			struct strbuf sb = STRBUF_INIT;
-			format_display(display_state, &sb, '-', _("[deleted]"), NULL,
-				       _("(none)"), ref->name,
-				       summary_width);
-			fputs(sb.buf, stderr);
-			strbuf_release(&sb);
+			display_ref_update(display_state, '-', _("[deleted]"), NULL,
+					   _("(none)"), ref->name,
+					   summary_width);
 			warn_dangling_symref(stderr, dangling_msg, ref->name);
 		}
 	}
-- 
2.40.0


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

  parent reply	other threads:[~2023-03-20 12:36 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-15 11:21 [PATCH 0/8] fetch: refactor code that prints reference updates Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 1/8] fetch: rename `display` buffer to avoid name conflict Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 2/8] fetch: move reference width calculation into `display_state` Patrick Steinhardt
2023-03-15 20:59   ` Junio C Hamano
2023-03-16 15:05     ` Patrick Steinhardt
2023-03-16 16:18       ` Junio C Hamano
2023-03-17 10:03         ` Patrick Steinhardt
2023-03-16 16:19       ` Junio C Hamano
2023-03-15 11:21 ` [PATCH 3/8] fetch: move output format " Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 4/8] fetch: pass the full local reference name to `format_display` Patrick Steinhardt
2023-03-15 22:18   ` Junio C Hamano
2023-03-15 11:21 ` [PATCH 5/8] fetch: deduplicate handling of per-reference format Patrick Steinhardt
2023-03-15 22:45   ` Junio C Hamano
2023-03-16 15:06     ` Patrick Steinhardt
2023-03-16 16:50       ` Junio C Hamano
2023-03-17  9:51         ` Patrick Steinhardt
2023-03-17 15:41           ` Junio C Hamano
2023-03-15 11:21 ` [PATCH 6/8] fetch: deduplicate logic to print remote URL Patrick Steinhardt
2023-03-15 23:02   ` Junio C Hamano
2023-03-16 15:06     ` Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 7/8] fetch: fix inconsistent summary width for pruned and updated refs Patrick Steinhardt
2023-03-15 23:12   ` Junio C Hamano
2023-03-16 15:06     ` Patrick Steinhardt
2023-03-16 16:30       ` Junio C Hamano
2023-03-17  9:55         ` Patrick Steinhardt
2023-03-15 11:21 ` [PATCH 8/8] fetch: centralize printing of reference updates Patrick Steinhardt
2023-03-17 20:24 ` [PATCH 0/8] fetch: refactor code that prints " Jonathan Tan
2023-03-20  6:57   ` Patrick Steinhardt
2023-03-20 12:26   ` Patrick Steinhardt
2023-03-20 12:35 ` [PATCH v2 0/6] " Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 1/6] fetch: move reference width calculation into `display_state` Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 2/6] fetch: move output format " Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 3/6] fetch: pass the full local reference name to `format_display` Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 4/6] fetch: centralize handling of per-reference format Patrick Steinhardt
2023-03-20 12:35   ` [PATCH v2 5/6] fetch: centralize logic to print remote URL Patrick Steinhardt
2023-03-20 12:35   ` Patrick Steinhardt [this message]
2023-03-20 22:57     ` [PATCH v2 6/6] fetch: centralize printing of reference updates Jonathan Tan
2023-03-22  9:04       ` Patrick Steinhardt
2023-03-29 18:45       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fe7e2e85eb37cd4068b5160721663c21a16a8138.1679315383.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.