Re: [PATCH 2/6] builtin/repo: humanise count values in structure output
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Dec 10, 2025, 06:28 UTC
- Message-ID
- <aTkS_kBlNsnbPyP5@pks.im>
- In-Reply-To
- <20251209225820.2861276-3-jltobler@gmail.com>
On Tue, Dec 09, 2025 at 04:58:16PM -0600, Justin Tobler wrote:
Show 15 quoted lines
> diff --git a/builtin/repo.c b/builtin/repo.c
> index a69699857a..8fb728b3a5 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -266,6 +275,10 @@ static void stats_table_addf(struct stats_table *table, const char *format, ...)
> va_end(ap);
> }
>
> +static const char *unit_k = "k";
> +static const char *unit_M = "M";
> +static const char *unit_G = "G";
> +
> static void stats_table_count_addf(struct stats_table *table, size_t value,
> const char *format, ...)
> {I would assume that these units should be translatable.
Show 28 quoted lines
> @@ -273,7 +286,26 @@ static void stats_table_count_addf(struct stats_table *table, size_t value,
> va_list ap;
>
> CALLOC_ARRAY(entry, 1);
> - entry->value = xstrfmt("%" PRIuMAX, (uintmax_t)value);
> +
> + if (value >= 1000000000) {
> + uintmax_t x = (uintmax_t)value + 5000000;
> + entry->value = xstrfmt("%" PRIuMAX ".%02" PRIuMAX,
> + x / 1000000000,
> + x % 1000000000 / 10000000);
> + entry->unit = unit_G;
> + } else if (value >= 1000000) {
> + uintmax_t x = (uintmax_t)value + 5000;
> + entry->value = xstrfmt("%" PRIuMAX ".%02" PRIuMAX,
> + x / 1000000, x % 1000000 / 10000);
> + entry->unit = unit_M;
> + } else if (value >= 1000) {
> + uintmax_t x = (uintmax_t)value + 5;
> + entry->value = xstrfmt("%" PRIuMAX ".%02" PRIuMAX,
> + x / 1000, x % 1000 / 10);
> + entry->unit = unit_k;
> + } else {
> + entry->value = xstrfmt("%" PRIuMAX, (uintmax_t)value);
> + }
>
> va_start(ap, format);
> stats_table_vaddf(table, entry, format, ap);These units are decimal-based (1000), whereas in "parse.c" we have `get_unit_factor()` that is binary-based (1024). Arguably, it's "parse.c" that is wrong because "k" is generally decimal-based whereas "Ki" would be binary-based.
Not quite sure what to do with this. For counts it _could_ be okay if we continue to use the wrong unit prefix. But as soon as we get to disk sizes we certainly should use the correct units, which would probably be KiB.
Show 41 quoted lines
> diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh > index 36a71a144e..55fd13ad1b 100755 > --- a/t/t1901-repo-structure.sh > +++ b/t/t1901-repo-structure.sh > @@ -10,21 +10,21 @@ test_expect_success 'empty repository' ' > ( > cd repo && > cat >expect <<-\EOF && > - | Repository structure | Value | > - | -------------------- | ----- | > - | * References | | > - | * Count | 0 | > - | * Branches | 0 | > - | * Tags | 0 | > - | * Remotes | 0 | > - | * Others | 0 | > - | | | > - | * Reachable objects | | > - | * Count | 0 | > - | * Commits | 0 | > - | * Trees | 0 | > - | * Blobs | 0 | > - | * Tags | 0 | > + | Repository structure | Value | > + | -------------------- | ------ | > + | * References | | > + | * Count | 0 | > + | * Branches | 0 | > + | * Tags | 0 | > + | * Remotes | 0 | > + | * Others | 0 | > + | | | > + | * Reachable objects | | > + | * Count | 0 | > + | * Commits | 0 | > + | * Trees | 0 | > + | * Blobs | 0 | > + | * Tags | 0 | > EOF > > git repo structure >out 2>err &&
It's a bit weird that this test here changes even though we don't even use any units. But I don't mind it too much.
Patrick