Jeff King <peff@xxxxxxxx> writes:
> 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).
The former might be conceivable, but I do not think the latter is
workable. Knowing that a location happened to be using a particular
GMT offset at a specific point in time simply is not sufficient to
give us a zone name; the whole idea of a zone name being that it
will give us rules that would apply to other timestamps, not just
the one we are paring with GMT offset in our committer and author
timestamp fields.
A third possibility is the information may come out of band.
Something like "When gitster is in +0900 he is in JST" can be
maintained per project and supplied by the caller.
> 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.
I agree that it is not too much hassle to revert this change. I
actually would not have minded if René's original were written with
a boolean flag. But I do not see the value in flipping ("" vs NULL)
with a bool now. The benefit we are gaining (other than closing the
door) is unclear to me.
>> /**
>> * 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.
Yeah, that reads better. I am also OK if this said that an empty
string is an accepted POSIX way to fallback when the information is
unavailable---and we really are in that "information is unavailable"
situation in this code.
