Re: [PATCH v2 03/10] builtin/cat-file: support "blob:none" objects filter
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Apr 1, 2025, 12:22 UTC
- Message-ID
- <CAOLa=ZR4YmfHvQGvs1mzdtRu5z+8XW-xY+t_wBAoTs1m+rrO9A@mail.gmail.com>
- In-Reply-To
- <20250327-pks-cat-file-object-type-filter-v2-3-4bbc7085d7c5@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 36 quoted lines
> Implement support for the "blob:none" filter in git-cat-file(1), which
> causes us to omit all blobs.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Documentation/git-cat-file.adoc | 2 ++
> builtin/cat-file.c | 11 ++++++++++-
> t/t1006-cat-file.sh | 33 +++++++++++++++++++++++++++++++--
> 3 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
> index f7f57b7f538..bb32f715944 100644
> --- a/Documentation/git-cat-file.adoc
> +++ b/Documentation/git-cat-file.adoc
> @@ -86,6 +86,8 @@ OPTIONS
> Omit objects from the list of printed objects. This can only be used in
> combination with one of the batched modes. The '<filter-spec>' may be
> one of the following:
> ++
> +The form '--filter=blob:none' omits all blobs.
>
> --path=<path>::
> For use with `--textconv` or `--filters`, to allow specifying an object
> 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]