Re: [PATCH] list-objects-filter: merge filter data structs

> Matthew DeVore <matvore@comcast.net> writes:
> 
>> Simplify the filter execution data logic and structs by putting all
>> execution data for all filter types in a single struct. This results in
>> a tiny overhead for each filter instance, and in exchange, invoking
>> filters is not only easier but the list-objects-filter public API is
>> simpler and more opaque.
> 
> Hmmm...
> 
>> +struct filter_data {
>> +	/* Used by all filter types. */
>>   	struct oidset *omits;
>> +
>> +	enum list_objects_filter_result (*filter_object_fn)(
>> +		struct repository *r,
>> +		enum list_objects_filter_situation filter_situation,
>> +		struct object *obj,
>> +		const char *pathname,
>> +		const char *filename,
>> +		struct filter_data *filter_data);
>> +
>> +	/* BEGIN tree:<depth> filter data */
>> +
>> +	/*
>> +	 * Maps trees to the minimum depth at which they were seen. It is not
>> +	 * necessary to re-traverse a tree at deeper or equal depths than it has
>> +	 * already been traversed.
>> +	 *
>> +	 * We can't use LOFR_MARK_SEEN for tree objects since this will prevent
>> +	 * it from being traversed at shallower depths.
>> +	 */
>> +	struct oidmap seen_at_depth;
>> +
>> +	unsigned long exclude_depth;
>> +	unsigned long current_depth;
>> +
>> +	/* BEGIN blobs:limit=<limit> filter data */
>> +
>> +	unsigned long max_bytes;
>> +
>> +	/* BEGIN sparse:... filter data */
>> +
>> +	struct exclude_list el;
>> +
>> +	size_t nr, alloc;
>> +	struct frame *array_frame;
>>   };
> 
> I am hoping that I am not misreading the intention but you do not
> plan to use the above so that you can say "apply 'tree:depth=4' and
> 'blobs:limit=1G' at the same time" by filling the fields in a single
> struct, do you?  For combined filter, you'll still have multiple
> instances of filter_data struct, strung together in a list that says
> "all of these must be satisfied" or something like that, right?
> 
> And if that is the case, I am not sure why the above "struct with
> all these fields" is a good idea.  If these three (and probably we
> will have more as the system evolves) sets of fields in this outer
> struct for different filters were enclosed in a union, that would be
> a different story, though.
>