Re: [PATCH 4/4] builtin/repo: add nul format for stats

> On Mon, Sep 22, 2025 at 09:57:00PM -0500, Justin Tobler wrote:
> > diff --git a/builtin/repo.c b/builtin/repo.c
> > index 4c16a68e4e..37034e6347 100644
> > --- a/builtin/repo.c
> > +++ b/builtin/repo.c
> > @@ -291,27 +291,31 @@ static void stats_table_print(struct stats_table *table)
> >  	strbuf_release(&buf);
> >  }
> >  
> > -static void stats_print(struct stats *stats)
> > +static void stats_print(struct stats *stats, int nul_delim)
> 
> Instead of passing a boolean-style option, can't we pass the expected
> delimiter directly? Makes the callsite a bit more obvious.

> >  {
> >  	struct strbuf buf = STRBUF_INIT;
> > -
> > -	strbuf_addf(&buf, "references.branches.count=%" PRIuMAX "\n",
> > -		    (uintmax_t)stats->refs.branches);
> > -	strbuf_addf(&buf, "references.tags.count=%" PRIuMAX "\n",
> > -		    (uintmax_t)stats->refs.tags);
> > -	strbuf_addf(&buf, "references.remotes.count=%" PRIuMAX "\n",
> > -		    (uintmax_t)stats->refs.remotes);
> > -	strbuf_addf(&buf, "references.others.count=%" PRIuMAX "\n",
> > -		    (uintmax_t)stats->refs.others);
> > -
> > -	strbuf_addf(&buf, "objects.commits.count=%" PRIuMAX "\n",
> > -		    (uintmax_t)stats->objects.commits);
> > -	strbuf_addf(&buf, "objects.trees.count=%" PRIuMAX "\n",
> > -		    (uintmax_t)stats->objects.trees);
> > -	strbuf_addf(&buf, "objects.blobs.count=%" PRIuMAX "\n",
> > -		    (uintmax_t)stats->objects.blobs);
> > -	strbuf_addf(&buf, "objects.tags.count=%" PRIuMAX "\n",
> > -		    (uintmax_t)stats->objects.tags);
> > +	char delim = '\n';
> > +
> > +	if (nul_delim)
> > +		delim = '\0';
> > +
> > +	strbuf_addf(&buf, "references.branches.count=%" PRIuMAX "%c",
> > +		    (uintmax_t)stats->refs.branches, delim);
> > +	strbuf_addf(&buf, "references.tags.count=%" PRIuMAX "%c",
> > +		    (uintmax_t)stats->refs.tags, delim);
> > +	strbuf_addf(&buf, "references.remotes.count=%" PRIuMAX "%c",
> > +		    (uintmax_t)stats->refs.remotes, delim);
> > +	strbuf_addf(&buf, "references.others.count=%" PRIuMAX "%c",
> > +		    (uintmax_t)stats->refs.others, delim);
> > +
> > +	strbuf_addf(&buf, "objects.commits.count=%" PRIuMAX "%c",
> > +		    (uintmax_t)stats->objects.commits, delim);
> > +	strbuf_addf(&buf, "objects.trees.count=%" PRIuMAX "%c",
> > +		    (uintmax_t)stats->objects.trees, delim);
> > +	strbuf_addf(&buf, "objects.blobs.count=%" PRIuMAX "%c",
> > +		    (uintmax_t)stats->objects.blobs, delim);
> > +	strbuf_addf(&buf, "objects.tags.count=%" PRIuMAX "%c",
> > +		    (uintmax_t)stats->objects.tags, delim);
> >  
> >  	fwrite(buf.buf, sizeof(char), buf.len, stdout);
> >  	strbuf_release(&buf);
> 
> It's a bit unfortunate we have to rewrite most of the function. I'd
> either have the `delim` parameter right from the start or just squash
> these two patches together.

> > diff --git a/t/t1901-repo-stats.sh b/t/t1901-repo-stats.sh
> > index 5bc6d9d5c4..061b2fbbc1 100755
> > --- a/t/t1901-repo-stats.sh
> > +++ b/t/t1901-repo-stats.sh
> > @@ -127,4 +127,31 @@ test_expect_success 'repository stats with keyvalue format' '
> >  	)
> >  '
> >  
> > +test_expect_success 'repository stats with nul format' '
> > +	test_when_finished "rm -rf repo" &&
> > +	git init repo &&
> > +	(
> > +		cd repo &&
> > +		test_commit_bulk 42 &&
> > +		git tag -a foo -m bar &&
> > +		git repo stats --format=nul >out 2>err &&
> > +
> > +		cat >expect <<-EOF &&
> > +		references.branches.count=1
> > +		references.tags.count=1
> > +		references.remotes.count=0
> > +		references.others.count=0
> > +		objects.commits.count=42
> > +		objects.trees.count=42
> > +		objects.blobs.count=42
> > +		objects.tags.count=1
> > +		EOF
> > +
> > +		tr "\n" "\0" <expect >expect_null &&
> > +
> > +		test_cmp expect_null out &&
> > +		test_line_count = 0 err
> > +	)
> > +'
> 
> We already have a test for the keyvalue format that looks mostly the
> same, so we may just as well test both formats in a single test.