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

[Jeff King <peff@peff.net>]

On Fri, Jun 23, 2017 at 05:13:38PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > The idea was that eventually the caller might be able to come up with a
> > TZ that is not blank, but is also not what strftime("%Z") would produce.
> > Conceivably that could be done if Git commits carried the "%Z"
> > information (not likely), or if we used a reverse-lookup table (also not
> > likely).
> >
> > This closes the door on that.  Since we don't have immediate plans to go
> > that route, I'm OK with this patch. It would be easy enough to re-open
> > the door if we change our minds later.
> 
> Closes the door on doing that via passing the char * of the prepared
> custom tz_name to strbuf_addftime().
> 
> 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.

> >>   * Add the time specified by `tm`, as formatted by `strftime`.
> >> - * `tz_name` is used to expand %Z internally unless it's NULL.
> >>   * `tz_offset` is in decimal hhmm format, e.g. -600 means six hours west
> >>   * of Greenwich, and it's used to expand %z internally.  However, tokens
> >>   * with modifiers (e.g. %Ez) are passed to `strftime`.
> >> + * `omit_strftime_tz_name` when set, means don't let `strftime` format
> >> + * %Z, instead do our own formatting.
> >
> > Since we now always turn it into a blank string, perhaps "do our own
> > formatting" could be more descriptive: we convert it into the empty
> > string.
> 
> Then we'd need to change this comment again if we had some patch like
> the one I mentioned above, I thought it was better to just leave this
> vague enough that we didn't need to do that.

Right, if you're going to do your own formatting inside the function,
then I agree the wording should be kept. But then "omit" is not really
the right word. Isn't it "tzname_from_tz" or something?

-Peff