On Tue, Apr 01, 2025 at 05:31:24AM -0700, Karthik Nayak wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> > Patrick Steinhardt <ps@xxxxxx> 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
