Re: [PATCH 08/11] bundle: parse filter capability

[Derrick Stolee <derrickstolee@github.com>]

On 3/7/2022 10:38 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:
> 
>> From: Derrick Stolee <derrickstolee@github.com>
...
>> diff --git a/bundle.h b/bundle.h
>> index 06009fe6b1f..eb026153d56 100644
>> --- a/bundle.h
>> +++ b/bundle.h
>> @@ -5,11 +5,14 @@
>>  #include "cache.h"
>>  #include "string-list.h"
>>  
>> +struct list_objects_filter_options;
>> +
> 
> For the other ones we include the relevant header, do the same here (or
> if there's a need to not do it, do we need it for the rest too?)

The others are .c files that require looking into the struct. This
declaration is all that's required for this header file.

>>  struct bundle_header {
>>  	unsigned version;
>>  	struct string_list prerequisites;
>>  	struct string_list references;
>>  	const struct git_hash_algo *hash_algo;
>> +	struct list_objects_filter_options *filter;
>>  };
> 
> I haven't tried, but any reason this needs to be a *filter
> v.s. embedding it in the struct?
> 
> Then we'd just need list_objects_filter_release() and not the free() as
> well.
> 
> Is it because you're piggy-backing on "if (header->filter)" as "do we
> have it" state, better to check .nr?

Yes. I replied to Junio before that there is some assumption in
the filtering code that the .nr == 0 case is listed as a BUG()
so we would possibly be breaking expectations in a different
way doing the embedded version.

>> @@ -55,7 +55,7 @@ const char *list_object_filter_config_name(enum list_objects_filter_choice c)
>>   * expand_list_objects_filter_spec() first).  We also "intern" the arg for the
>>   * convenience of the current command.
>>   */
> 
> These API docs....
> 
>> -static int gently_parse_list_objects_filter(
>> +int gently_parse_list_objects_filter(
>>  	struct list_objects_filter_options *filter_options,
>>  	const char *arg,
>>  	struct strbuf *errbuf)
>> diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
>> index da5b6737e27..347a99c28cf 100644
>> --- a/list-objects-filter-options.h
>> +++ b/list-objects-filter-options.h
>> @@ -72,6 +72,11 @@ struct list_objects_filter_options {
>>  /* Normalized command line arguments */
>>  #define CL_ARG__FILTER "filter"
> 
> ...should be moved here, presumably.

Yes. Thanks!
-Stolee