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

[Patrick Steinhardt <ps@pks.im>]

On Tue, Sep 23, 2025 at 10:33:45AM -0500, Justin Tobler wrote:
> On 25/09/23 12:53PM, Patrick Steinhardt wrote:
> > 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.
> 
> Ya, we could do that here instead. Something I just noticed is that the
> NUL format in `git repo info` also replaces the '=' delimiter with a
> newline. I'm not sure if it would be best to match the same behavior
> here?
> 
> If so, we would have to either pass both delimiters as arguments to the
> function, or just keep the boolean toggle for the mode.

I think being consistent would be nice, yes. I'd personally lean towards
passing both delimiters as arguments in that case. Makes the callsites
way easier to read.

Patrick