Matthew DeVore <matvore@xxxxxxxxxxx> 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.