Re: [PATCH] ref-filter: share bases and is_base_tips between formatting and sorting

From

Jeff King <peff@peff.net>

Date

Jan 16, 2025, 10:06 UTC

Message-ID

<20250116100637.GB144555@coredump.intra.peff.net>

In-Reply-To

<20250116095128.GA144555@coredump.intra.peff.net>

On Thu, Jan 16, 2025 at 04:51:28AM -0500, Jeff King wrote:

> Yet another option in the near term might be storing these ahead-behind
> bits in the individual atoms. Since the point is to do a single
> traversal, we'd have to marshal them into a unified data structure at
> some point. But we already do that! In filter_ahead_behind() we convert
> the string list into an array (and ironically do not even look at the
> strings, only their "util" fields).
> 
> So something like this (only lightly tested) seems to work:

diff --git a/builtin/branch.c b/builtin/branch.c
index 6e7b0cfddb..fbb9536282 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -473,7 +473,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 	if (verify_ref_format(format))
 		die(_("unable to parse format string"));
 
-	filter_ahead_behind(the_repository, format, &array);
+	filter_ahead_behind(the_repository, &array);
 	ref_array_sort(sorting, &array);
 
 	if (column_active(colopts)) {
diff --git a/ref-filter.c b/ref-filter.c
index 4c10b6fe75..0de51f13e6 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -894,7 +894,7 @@ static int rest_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int ahead_behind_atom_parser(struct ref_format *format,
+static int ahead_behind_atom_parser(struct ref_format *format UNUSED,
 				    struct used_atom *atom,
 				    const char *arg, struct strbuf *err)
 {
@@ -3084,7 +3084,6 @@ static void reach_filter(struct ref_array *array,
 }
 
 void filter_ahead_behind(struct repository *r,
-			 struct ref_format *format,
 			 struct ref_array *array)
 {
 	struct commit **commits;
@@ -3319,7 +3318,7 @@ void filter_and_format_refs(struct ref_filter *filter, unsigned int type,
 	} else {
 		struct ref_array array = { 0 };
 		filter_refs(&array, filter, type);
-		filter_ahead_behind(the_repository, format, &array);
+		filter_ahead_behind(the_repository, &array);
 		filter_is_base(the_repository, format, &array);
 		ref_array_sort(sorting, &array);
 		print_formatted_ref_array(&array, format);
diff --git a/ref-filter.h b/ref-filter.h
index d048317802..5f3dd6c931 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -201,7 +201,6 @@ struct ref_array_item *ref_array_push(struct ref_array *array,
  * If this is not called, then any ahead-behind atoms will be blank.
  */
 void filter_ahead_behind(struct repository *r,
-			 struct ref_format *format,
 			 struct ref_array *array);
 
 /*

Getting rid of the ref_format argument to filter_ahead_behind makes it
even more plain that we are operating on magic global data behind the
scenes. ;) So in that sense this is all the same step back that I was
complaining about from your patch; eventually we'd need to pass the
used_atom array to filter_ahead_behind() via some struct. But somehow it
seems less bad to me because it is all being folded into the existing
global structure.

-Peff