Re: [PATCH 5/9] builtin/cat-file: support "object:type=" objects filter

[Patrick Steinhardt <ps@pks.im>]

On Wed, Feb 26, 2025 at 04:23:12PM +0100, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index f57bf65cb03..b374c2bb104 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -474,7 +474,8 @@ static void batch_object_write(const char *obj_name,
> >  
> >  		if (use_mailmap ||
> >  		    opt->objects_filter.choice == LOFC_BLOB_NONE ||
> > -		    opt->objects_filter.choice == LOFC_BLOB_LIMIT)
> > +		    opt->objects_filter.choice == LOFC_BLOB_LIMIT ||
> > +		    opt->objects_filter.choice == LOFC_OBJECT_TYPE)
> >  			data->info.typep = &data->type;
> >  		if (opt->objects_filter.choice == LOFC_BLOB_LIMIT)
> >  			data->info.sizep = &data->size;
> > @@ -505,6 +506,10 @@ static void batch_object_write(const char *obj_name,
> >  			    data->size >= opt->objects_filter.blob_limit_value)
> >  				return;
> >  			break;
> > +		case LOFC_OBJECT_TYPE:
> > +			if (data->type != opt->objects_filter.object_type)
> > +				return;
> > +			break;
> >  		default:
> >  			BUG("unsupported objects filter");
> 
> I see we don't support LOFC_COMBINE, so we won't be supporting repeating
> the --filter= option, is this intentional? Should we support that too? I
> feel it would make sense from the start, unless there are good reasons
> not to?

I think the usefulness of LOFC_COMBINE is quite restricted in our case
because we only support a subset of filters in the first place. There is
only a single combination that does make sense: `blob:limit` plus
`object:type=blob`. All the other combinations are useless as they only
filter based on the object type, and thus they would yield the empty
set.

So given that this isn't that useful and given that it does add quite a
bit of complexity to support I decided to not support it for now, also
because I don't have any usecase for it.

Patrick