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

[PATCH v3 2/2] clean up interface for refs_warn_dangling_symrefs

From
Phil Hord <phil.hord@gmail.com>
Date
Jul 2, 2025, 00:58 UTC
Message-ID
<20250702005837.2813893-4-phil.hord@gmail.com>
In-Reply-To
<20250702005837.2813893-2-phil.hord@gmail.com>
From: Phil Hord <phil.hord@gmail.com>

The refs_warn_dangling_symrefs interface is a bit fragile as it passes in printf-formatting strings with expectations about the number of arguments. This patch series made it worse by adding a 2nd positional argument. But there are only two call sites, and they both use almost identical display options.

Make this safer by moving the format strings into the function that uses them to make it easier to see when the arguments don't match. Pass a prefix string and a dry_run flag so the decision logic can be handled where needed.

Signed-off-by: Phil Hord <phil.hord@gmail.com>
---
 builtin/fetch.c  |  5 +----
 builtin/remote.c |  5 +----
 refs.c           | 17 +++++++++++------
 refs.h           |  3 ++-
 4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 04d10c9e781a..fc72f2119c56 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1384,9 +1384,6 @@ static int prune_refs(struct display_state *display_state,
 	struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map);
 	struct strbuf err = STRBUF_INIT;
 	struct string_list refnames = STRING_LIST_INIT_NODUP;
-	const char *dangling_msg = dry_run
-		? _("   %s will become dangling after %s is deleted")
-		: _("   %s has become dangling after %s was deleted");
 
 	for (ref = stale_refs; ref; ref = ref->next)
 		string_list_append(&refnames, ref->name);
@@ -1417,7 +1414,7 @@ static int prune_refs(struct display_state *display_state,
 		}
 		string_list_sort(&refnames);
 		refs_warn_dangling_symrefs(get_main_ref_store(the_repository),
-					   stderr, dangling_msg, &refnames);
+					   stderr, "   ", dry_run, &refnames);
 	}
 
 cleanup:
diff --git a/builtin/remote.c b/builtin/remote.c
index 4de7dd373ae5..f672799e0d92 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1521,9 +1521,6 @@ static int prune_remote(const char *remote, int dry_run)
 	struct ref_states states = REF_STATES_INIT;
 	struct string_list refs_to_prune = STRING_LIST_INIT_NODUP;
 	struct string_list_item *item;
-	const char *dangling_msg = dry_run
-		? _(" %s will become dangling after %s is deleted!")
-		: _(" %s has become dangling after %s was deleted!");
 
 	get_remote_ref_states(remote, &states, GET_REF_STATES);
 
@@ -1555,7 +1552,7 @@ static int prune_remote(const char *remote, int dry_run)
 	}
 
 	refs_warn_dangling_symrefs(get_main_ref_store(the_repository),
-				   stdout, dangling_msg, &refs_to_prune);
+				   stdout, " ", dry_run, &refs_to_prune);
 
 	string_list_clear(&refs_to_prune, 0);
 	free_remote_ref_states(&states);
diff --git a/refs.c b/refs.c
index 07197c239e33..5602c18dbd5b 100644
--- a/refs.c
+++ b/refs.c
@@ -439,7 +439,8 @@ struct warn_if_dangling_data {
 	struct ref_store *refs;
 	FILE *fp;
 	const struct string_list *refnames;
-	const char *msg_fmt;
+	const char *indent;
+	int dry_run;
 };
 
 static int warn_if_dangling_symref(const char *refname, const char *referent UNUSED,
@@ -447,7 +448,7 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU
 				   int flags, void *cb_data)
 {
 	struct warn_if_dangling_data *d = cb_data;
-	const char *resolves_to;
+	const char *resolves_to, *msg;
 
 	if (!(flags & REF_ISSYMREF))
 		return 0;
@@ -458,19 +459,23 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU
 		return 0;
 	}
 
-	fprintf(d->fp, d->msg_fmt, refname, resolves_to);
-	fputc('\n', d->fp);
+	msg = d->dry_run
+		? _("%s%s will become dangling after %s is deleted\n")
+		: _("%s%s has become dangling after %s was deleted\n");
+	fprintf(d->fp, msg, d->indent, refname, resolves_to);
 	return 0;
 }
 
 void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,
-				const char *msg_fmt, const struct string_list *refnames)
+				const char *indent, int dry_run,
+				const struct string_list *refnames)
 {
 	struct warn_if_dangling_data data = {
 		.refs = refs,
 		.fp = fp,
 		.refnames = refnames,
-		.msg_fmt = msg_fmt,
+		.indent = indent,
+		.dry_run = dry_run,
 	};
 	refs_for_each_rawref(refs, warn_if_dangling_symref, &data);
 }
diff --git a/refs.h b/refs.h
index 07f21824d480..25bed4d792e7 100644
--- a/refs.h
+++ b/refs.h
@@ -453,7 +453,8 @@ static inline const char *has_glob_specials(const char *pattern)
 }
 
 void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,
-				const char *msg_fmt, const struct string_list *refnames);
+				const char *indent, int dry_run,
+				const struct string_list *refnames);
 
 /*
  * Flags for controlling behaviour of pack_refs()
-- 
2.50.0.149.g2f19833911.dirty
Previous: Phil HordNext: Junio C Hamano
Message 3 of 5 in “fetch --prune performance problem”
  1. 0/2 fetch --prune performance problemPhil Hord, Jul 2, 2025
  2. 1/2 refs: remove old refs_warn_dangling_symrefPhil Hord, Jul 2, 2025
  3. 2/2 clean up interface for refs_warn_dangling_symrefsPhil Hord, Jul 2, 2025
  4. Junio C HamanoJul 2, 2025
  5. Junio C HamanoJul 2, 2025

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.