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@xxxxxxxxxxxxxx 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.