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

> Am 24.06.2017 um 14:14 schrieb Ævar Arnfjörð Bjarmason:
>> Change the code for deciding what's to be done about %Z to stop
>> passing always either a NULL or "" char * to
>> strbuf_addftime(). Instead pass a boolean int to indicate whether the
>> strftime() %Z format should be suppressed by converting it to an empty
>> string, which is what this code is actually doing.
>>
>> This code grew organically between the changes in 9eafe86d58 ("Merge
>> branch 'rs/strbuf-addftime-zZ'", 2017-06-22). The intent was to use
>> this API in the future to pass a custom leave the door open to pass a
>> custom timezone name to the function (see my [1] and related
>> messages).
>
> "leave the door open to pass a" seems redundant.

>> But that's not what this code does now, and this strbuf_addstr() call
>> always being redundant makes it hard to understand the current
>> functionality. So simplify this internal API to match its use, we can
>> always change it in the future if it gets a different use-case.
>
> I don't understand the confusion, but of course I'm biased. And I don't
> like binary parameters in general and would use named flags or two
> function names in most cases.  But that aside I find the description
> hard to follow (perhaps I should do something about my attention span).

> Here's an attempt at a commit message that would have be easier to
> understand for me:
>
>   strbuf_addstr() allows callers to pass a time zone name for expanding
>   %Z.  The only current caller either passes the empty string or NULL,
>   in which case %Z is handed over verbatim to strftime(3).  Replace that
>   string parameter with a flag controlling whether to remove %Z from the
>   format specification.  This simplifies the code.