Re: [PATCH v3] strbuf: let strbuf_addftime handle %z and %Z itself

From: Jeff King <peff@peff.net>
Date: Jun 15, 2017, 13:49 UTC
Message-ID: <20170615134958.mzmdmhonjsnconu2@sigill.intra.peff.net>
In-Reply-To: <5f398cf6-2bc3-f999-acfa-0ef6d7a43435@web.de>

On Thu, Jun 15, 2017 at 02:29:53PM +0200, René Scharfe wrote:

Show 21 quoted lines

> There is no portable way to pass timezone information to strftime.  Add
> parameters for timezone offset and name to strbuf_addftime and let it
> handle the timezone-related format specifiers %z and %Z internally.
> 
> Callers can opt out for %Z by passing NULL as timezone name.  %z is
> always handled internally -- this helps on Windows, where strftime would
> expand it to a timezone name (same as %Z), in violation of POSIX.
> Modifiers are not handled, e.g. %Ez is still passed to strftime.
> 
> Use an empty string as timezone name in show_date (the only current
> caller) for now because we only have the timezone offset in non-local
> mode.  POSIX allows %Z to resolve to an empty string in case of missing
> information.
> 
> Helped-by: Ulrich Mueller <ulm@gentoo.org>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> Changes from v3:
> - Updated developer documentation in strbuf.h.
> - Added short note to user documentation.

This looks good to me overall.

Show 13 quoted lines

> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
> index 42d4ea61ef..71082008f0 100755
> --- a/t/t0006-date.sh
> +++ b/t/t0006-date.sh
> @@ -51,6 +51,12 @@ check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
>  check_show raw-local "$TIME" '1466000000 +0000'
>  check_show unix-local "$TIME" '1466000000'
>  
> +check_show 'format:%z' "$TIME" '+0200'
> +check_show 'format-local:%z' "$TIME" '+0000'
> +check_show 'format:%Z' "$TIME" ''
> +check_show 'format:%%z' "$TIME" '%z'
> +check_show 'format-local:%%z' "$TIME" '%z'

These check that the zone output is correct, but I don't think we ever check that the value we feed to strftime is actually in the correct zone in the first place (i.e., that %H shows the correct time).

I think that should go in a separate test from the %z/%Z handling, as there are some subtleties.

So here are two patches on top of yours: more tests, and then the format-local handling of %Z.

  [1/2]: t0006: check --date=format zone offsets
  [2/2]: date: use localtime() for "-local" time formats

 date.c          | 14 ++++++++++++--
 t/t0006-date.sh | 10 ++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

-Peff