Re: [PATCH] strbuf: change an always NULL/"" strbuf_addftime() param to bool

From: Jeff King <peff@peff.net>
Date: Jun 23, 2017, 16:37 UTC
Message-ID: <20170623163711.a3necnsqmroguuib@sigill.intra.peff.net>
In-Reply-To: <2e0773c3-b742-a211-d9cb-64da58cf9e0d@web.de>

On Fri, Jun 23, 2017 at 06:23:10PM +0200, René Scharfe wrote:

Show 12 quoted lines

> > > I have a WIP patch (which may not make it on-list, depending) playing
> > > with the idea I proposed in
> > > CACBZZX5OQc45fUyDVayE89rkT=+8m5S4efSXCAbCy7Upme5zLA@mail.gmail.com which
> > > just inserts the custom TZ name based on the offset inside that `if
> > > (omit_strftime_tz_name)` branch.
> > 
> > OK. I'd assumed that would all happen outside of strbuf_addftime(). But
> > if it happens inside, then I agree a flag is better.
> 
> Oh, so the interface that was meant to allow better time zone names
> without having to make strbuf_addftime() even bigger than it already is
> turns out to be too ugly for its purpose?  I'm sorry. :(

I haven't seen Ævar's patch, but I agree that if the caller did:

  if (mode->local)
	tzname = NULL; /* let strftime handle it */
  else
	tzname = fake_tz_from_offset(tz);
  ...
  strbuf_addftime(&buf, fmt, tm, tz, tzname);

that would be pretty clean (and what I was expecting with the "I'd assumed" above).

-Peff