From: Jeff King <peff@peff.net>
Date: Thu, 15 Feb 2018 05:56:19 GMT
Subject: Re: [PATCH v3 20/23] ref-filter: unifying formatting of cat-file opts
Message-ID: <20180215055618.GP18780@sigill.intra.peff.net>
In-Reply-To: <01020161890f437a-302b1577-f034-4a24-a3a2-851d1cda0044-000000@eu-west-1.amazonses.com>

On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:

> cat-file options are now filled by general logic.

Yay.

One puzzling thing:

> diff --git a/ref-filter.c b/ref-filter.c
> index 8d104b567eb7c..5781416cf9126 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -824,8 +824,12 @@ static void grab_common_values(struct atom_value *val, int deref, struct object
>  		else if (!strcmp(name, "objectsize")) {
>  			v->value = sz;
>  			v->s = xstrfmt("%lu", sz);
> -		}
> -		else if (deref)
> +		} else if (!strcmp(name, "objectsize:disk")) {
> +			if (cat_file_info.is_cat_file) {
> +				v->value = cat_file_info.disk_size;
> +				v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
> +			}
> +		} else if (deref)

Why do we care about is_cat_file here. Shouldn't:

  git for-each-ref --format='%(objectsize:disk)'

work? I.e., shouldn't the cat_file_info.disk_size variable be held
somewhere in a used_atom struct?

-Peff