Re: [PATCH v2 03/10] builtin/cat-file: support "blob:none" objects filter

[Patrick Steinhardt <ps@pks.im>]

On Tue, Apr 01, 2025 at 05:31:24AM -0700, Karthik Nayak wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
> > Patrick Steinhardt <ps@pks.im> writes:
> >> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> >> index 940900d92ad..e783dbbad58 100644
> >> --- a/builtin/cat-file.c
> >> +++ b/builtin/cat-file.c
> >> @@ -472,7 +472,8 @@ static void batch_object_write(const char *obj_name,
> >>  	if (!data->skip_object_info) {
> >>  		int ret;
> >>
> >> -		if (use_mailmap)
> >> +		if (use_mailmap ||
> >> +		    opt->objects_filter.choice == LOFC_BLOB_NONE)
> >>  			data->info.typep = &data->type;
> >>
> >
> > I didn't understand why we need to do this, below we only check for
> > `data->type`. The only other place we use `data->info.typep` going
> > forward seems to be `print_object_or_die()`, but that flow is only
> > followed for `opt->batch_mode == BATCH_MODE_CONTENTS`. We already have
> >
> >     if (opt->batch_mode == BATCH_MODE_CONTENTS)
> > 		data.info.typep = &data.type;
> >
> > in `batch_objects()` before this, shouldn't that cover this scenario
> > too? Maybe we can add a comment with the reasoning
> >
> > [snip]
> 
> After playing around more, I understand now, we set the pointer
> `data->info.typep` to point to `data->type`, so when the data is parsed
> in `packed_object_info()` or `oid_object_info_extended()`, that
> information would be set into `data->type`. So we can skip as needed.
> 
> All good here!

I've adapted the commit message to better explain this.

Patrick