Subject: Re: [PATCH 06/11] MyFirstObjectWalk: update recommended usage

From: Junio C Hamano <[email-masked]>
Date: Fri, 04 Mar 2022 14:33:34 -0800

"Derrick Stolee via GitGitGadget" <[email-masked]> writes:

> From: Derrick Stolee <[email-masked]>
>
> The previous change consolidated traverse_commit_list() and
> traverse_commit_list_filtered(). This allows us to simplify the
> recommended usage in MyFirstObjectWalk.txt to use this new set of
> values.
>
> While here, add some clarification on the difference between the two
> methods.
>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  Documentation/MyFirstObjectWalk.txt | 44 +++++++++++------------------
>  1 file changed, 16 insertions(+), 28 deletions(-)

Nice simplification.

>
> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt
> index ca267941f3e..8ec83185b8a 100644
> --- a/Documentation/MyFirstObjectWalk.txt
> +++ b/Documentation/MyFirstObjectWalk.txt
> @@ -522,24 +522,25 @@ function shows that the all-object walk is being performed by
>  `traverse_commit_list()` or `traverse_commit_list_filtered()`. Those two
>  functions reside in `list-objects.c`; examining the source shows that, despite
>  the name, these functions traverse all kinds of objects. Let's have a look at
> -the arguments to `traverse_commit_list_filtered()`, which are a superset of the
> -arguments to the unfiltered version.
> +the arguments to `traverse_commit_list()`.
>  
> -- `struct list_objects_filter_options *filter_options`: This is a struct which
> -  stores a filter-spec as outlined in `Documentation/rev-list-options.txt`.
> -- `struct rev_info *revs`: This is the `rev_info` used for the walk.
> +- `struct rev_info *revs`: This is the `rev_info` used for the walk. It
> +  includes a `filter` member which contains information for how to filter
> +  the object list.

Perhaps,

    "When its `filter` member is not NULL, it contains ..."

implying that it is valid for `filter` member to be NULL and none of
the following things will happen in such a case.

>  For now, we are not going to track the omitted objects, so we'll replace those
>  parameters with `NULL`. For the sake of simplicity, we'll add a simple
> -build-time branch to use our filter or not. Replace the line calling
> +build-time branch to use our filter or not. Preface the line calling

Good eyes.

>  `traverse_commit_list()` with the following, which will remind us which kind of
>  walk we've just performed:
>  
> @@ -733,19 +723,17 @@ walk we've just performed:
>  	if (0) {
>  		/* Unfiltered: */
>  		trace_printf(_("Unfiltered object walk.\n"));
> -		traverse_commit_list(rev, walken_show_commit,
> -				walken_show_object, NULL);
>  	} else {
>  		trace_printf(
>  			_("Filtered object walk with filterspec 'tree:1'.\n"));
> -		parse_list_objects_filter(&filter_options, "tree:1");
> -
> -		traverse_commit_list_filtered(&filter_options, rev,
> -			walken_show_commit, walken_show_object, NULL, NULL);
> +		CALLOC_ARRAY(rev->filter, 1);
> +		parse_list_objects_filter(rev->filter, "tree:1");
>  	}
> +	traverse_commit_list(rev, walken_show_commit,
> +			     walken_show_object, NULL);
>  ----
>  
> -`struct list_objects_filter_options` is usually built directly from a command
> +The `rev->filter` member is usually built directly from a command
>  line argument, so the module provides an easy way to build one from a string.
>  Even though we aren't taking user input right now, we can still build one with
>  a hardcoded string using `parse_list_objects_filter()`.
> @@ -784,7 +772,7 @@ object:
>  ----
>  	...
>  
> -		traverse_commit_list_filtered(&filter_options, rev,
> +		traverse_commit_list_filtered(rev,
>  			walken_show_commit, walken_show_object, NULL, &omitted);
>  
>  	...