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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Jun 23 2017, René Scharfe jotted:

> Am 23.06.2017 um 17:23 schrieb Jeff King:
>> 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.
>
> 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 don't think it's ugly. My motivation for sending this patch is that I
started playing with this code and was confused because I thought that
strbuf_addstr(...) actually did something to the string, but it never
did.

Since it's a purely internal API used in just one place I thought it
made sense to adjust the prototype / code to its current usage for ease
of readability, if we want to do something else with it in the future
it'll be trivial to adjust it then.

But I don't feel strongly about this patch at all, it's just a minor
fixup I submitted while reading / playing with the code.