On Tue, Nov 07, 2023 at 01:25:58AM +0000, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@xxxxxxxxxx>
>
> Factor out parts of 'ref_array_push()', 'ref_filter_handler()', and
> 'filter_refs()' into new helper functions ('ref_array_append()',
> 'apply_ref_filter()', and 'do_filter_refs()' respectively), as well as
> rename 'ref_filter_handler()' to 'filter_one()'. In this and later
> patches, these helpers will be used by new ref-filter API functions. This
> patch does not result in any user-facing behavior changes or changes to
> callers outside of 'ref-filter.c'.
>
> The changes are as follows:
>
> * The logic to grow a 'struct ref_array' and append a given 'struct
> ref_array_item *' to it is extracted from 'ref_array_push()' into
> 'ref_array_append()'.
> * 'ref_filter_handler()' is renamed to 'filter_one()' to more clearly
> distinguish it from other ref filtering callbacks that will be added in
> later patches. The "*_one()" naming convention is common throughout the
> codebase for iteration callbacks.
> * The code to filter a given ref by refname & object ID then create a new
> 'struct ref_array_item' is moved out of 'filter_one()' and into
> 'apply_ref_filter()'. 'apply_ref_filter()' returns either NULL (if the ref
> does not match the given filter) or a 'struct ref_array_item *' created
> with 'new_ref_array_item()'; 'filter_one()' appends that item to
> its ref array with 'ref_array_append()'.
> * The filter pre-processing, contains cache creation, and ref iteration of
> 'filter_refs()' is extracted into 'do_filter_refs()'. 'do_filter_refs()'
> takes its ref iterator function & callback data as an input from the
> caller, setting it up to be used with additional filtering callbacks in
> later patches.
To me, a bulleted list spelling out the different changes I'm doing
often indicates that I might want to split up the commit into one for
each of the items. I don't feel strongly about this, but think that it
might help the reviewer in this case.
Patrick
> Signed-off-by: Victoria Dye <vdye@xxxxxxxxxx>
> ---
> ref-filter.c | 115 ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 69 insertions(+), 46 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 8992fbf45b1..ff00ab4b8d8 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -2716,15 +2716,18 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
> return ref;
> }
>
> +static void ref_array_append(struct ref_array *array, struct ref_array_item *ref)
> +{
> + ALLOC_GROW(array->items, array->nr + 1, array->alloc);
> + array->items[array->nr++] = ref;
> +}
> +
> struct ref_array_item *ref_array_push(struct ref_array *array,
> const char *refname,
> const struct object_id *oid)
> {
> struct ref_array_item *ref = new_ref_array_item(refname, oid);
> -
> - ALLOC_GROW(array->items, array->nr + 1, array->alloc);
> - array->items[array->nr++] = ref;
> -
> + ref_array_append(array, ref);
> return ref;
> }
>
> @@ -2761,46 +2764,36 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
> return ref_kind_from_refname(refname);
> }
>
> -struct ref_filter_cbdata {
> - struct ref_array *array;
> - struct ref_filter *filter;
> -};
> -
> -/*
> - * A call-back given to for_each_ref(). Filter refs and keep them for
> - * later object processing.
> - */
> -static int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
> +static struct ref_array_item *apply_ref_filter(const char *refname, const struct object_id *oid,
> + int flag, struct ref_filter *filter)
> {
> - struct ref_filter_cbdata *ref_cbdata = cb_data;
> - struct ref_filter *filter = ref_cbdata->filter;
> struct ref_array_item *ref;
> struct commit *commit = NULL;
> unsigned int kind;
>
> if (flag & REF_BAD_NAME) {
> warning(_("ignoring ref with broken name %s"), refname);
> - return 0;
> + return NULL;
> }
>
> if (flag & REF_ISBROKEN) {
> warning(_("ignoring broken ref %s"), refname);
> - return 0;
> + return NULL;
> }
>
> /* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */
> kind = filter_ref_kind(filter, refname);
> if (!(kind & filter->kind))
> - return 0;
> + return NULL;
>
> if (!filter_pattern_match(filter, refname))
> - return 0;
> + return NULL;
>
> if (filter_exclude_match(filter, refname))
> - return 0;
> + return NULL;
>
> if (filter->points_at.nr && !match_points_at(&filter->points_at, oid, refname))
> - return 0;
> + return NULL;
>
> /*
> * A merge filter is applied on refs pointing to commits. Hence
> @@ -2811,15 +2804,15 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
> filter->with_commit || filter->no_commit || filter->verbose) {
> commit = lookup_commit_reference_gently(the_repository, oid, 1);
> if (!commit)
> - return 0;
> + return NULL;
> /* We perform the filtering for the '--contains' option... */
> if (filter->with_commit &&
> !commit_contains(filter, commit, filter->with_commit, &filter->internal.contains_cache))
> - return 0;
> + return NULL;
> /* ...or for the `--no-contains' option */
> if (filter->no_commit &&
> commit_contains(filter, commit, filter->no_commit, &filter->internal.no_contains_cache))
> - return 0;
> + return NULL;
> }
>
> /*
> @@ -2827,11 +2820,32 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
> * to do its job and the resulting list may yet to be pruned
> * by maxcount logic.
> */
> - ref = ref_array_push(ref_cbdata->array, refname, oid);
> + ref = new_ref_array_item(refname, oid);
> ref->commit = commit;
> ref->flag = flag;
> ref->kind = kind;
>
> + return ref;
> +}
> +
> +struct ref_filter_cbdata {
> + struct ref_array *array;
> + struct ref_filter *filter;
> +};
> +
> +/*
> + * A call-back given to for_each_ref(). Filter refs and keep them for
> + * later object processing.
> + */
> +static int filter_one(const char *refname, const struct object_id *oid, int flag, void *cb_data)
> +{
> + struct ref_filter_cbdata *ref_cbdata = cb_data;
> + struct ref_array_item *ref;
> +
> + ref = apply_ref_filter(refname, oid, flag, ref_cbdata->filter);
> + if (ref)
> + ref_array_append(ref_cbdata->array, ref);
> +
> return 0;
> }
>
> @@ -2967,26 +2981,12 @@ void filter_ahead_behind(struct repository *r,
> free(commits);
> }
>
> -/*
> - * API for filtering a set of refs. Based on the type of refs the user
> - * has requested, we iterate through those refs and apply filters
> - * as per the given ref_filter structure and finally store the
> - * filtered refs in the ref_array structure.
> - */
> -int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type)
> +static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref_fn fn, void *cb_data)
> {
> - struct ref_filter_cbdata ref_cbdata;
> - int save_commit_buffer_orig;
> int ret = 0;
>
> - ref_cbdata.array = array;
> - ref_cbdata.filter = filter;
> -
> filter->kind = type & FILTER_REFS_KIND_MASK;
>
> - save_commit_buffer_orig = save_commit_buffer;
> - save_commit_buffer = 0;
> -
> init_contains_cache(&filter->internal.contains_cache);
> init_contains_cache(&filter->internal.no_contains_cache);
>
> @@ -3001,20 +3001,43 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
> * of filter_ref_kind().
> */
> if (filter->kind == FILTER_REFS_BRANCHES)
> - ret = for_each_fullref_in("refs/heads/", ref_filter_handler, &ref_cbdata);
> + ret = for_each_fullref_in("refs/heads/", fn, cb_data);
> else if (filter->kind == FILTER_REFS_REMOTES)
> - ret = for_each_fullref_in("refs/remotes/", ref_filter_handler, &ref_cbdata);
> + ret = for_each_fullref_in("refs/remotes/", fn, cb_data);
> else if (filter->kind == FILTER_REFS_TAGS)
> - ret = for_each_fullref_in("refs/tags/", ref_filter_handler, &ref_cbdata);
> + ret = for_each_fullref_in("refs/tags/", fn, cb_data);
> else if (filter->kind & FILTER_REFS_ALL)
> - ret = for_each_fullref_in_pattern(filter, ref_filter_handler, &ref_cbdata);
> + ret = for_each_fullref_in_pattern(filter, fn, cb_data);
> if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))
> - head_ref(ref_filter_handler, &ref_cbdata);
> + head_ref(fn, cb_data);
> }
>
> clear_contains_cache(&filter->internal.contains_cache);
> clear_contains_cache(&filter->internal.no_contains_cache);
>
> + return ret;
> +}
> +
> +/*
> + * API for filtering a set of refs. Based on the type of refs the user
> + * has requested, we iterate through those refs and apply filters
> + * as per the given ref_filter structure and finally store the
> + * filtered refs in the ref_array structure.
> + */
> +int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type)
> +{
> + struct ref_filter_cbdata ref_cbdata;
> + int save_commit_buffer_orig;
> + int ret = 0;
> +
> + ref_cbdata.array = array;
> + ref_cbdata.filter = filter;
> +
> + save_commit_buffer_orig = save_commit_buffer;
> + save_commit_buffer = 0;
> +
> + ret = do_filter_refs(filter, type, filter_one, &ref_cbdata);
> +
> /* Filters that need revision walking */
> reach_filter(array, &filter->reachable_from, INCLUDE_REACHED);
> reach_filter(array, &filter->unreachable_from, EXCLUDE_REACHED);
> --
> gitgitgadget
>
>
Attachment:
signature.asc
Description: PGP signature
