jq icon indicating copy to clipboard operation
jq copied to clipboard

Fix strflocaltime on daylight saving time values

Open wjlroe opened this issue 4 years ago • 7 comments

This PR changes the way time values get serialised and deserialised to the internal format when passing them between stages. Previously the internal value didn't retain the dst value so time values would lose that field and when printed using strflocaltime, the fact the value was in DST was lost. This seems to me to be the only way to have strflocaltime printing the correct time zone information on values during a daylight saving time.

The following behaviour demonstrates how the daylight savings value gets lost when a time value gets turned into an internal array of numbers and then back into a (struct tm):

    $ TZ=Europe/London ./jq -r -n '1504803635 | strflocaltime("%Y-%m-%d %H:%M:%S (%Z)")'
    2017-09-07 18:00:35 (GMT)

After this change however, this works as expected:

    $ TZ=Europe/London ./jq -r -n '1504803635 | strflocaltime("%Y-%m-%d %H:%M:%S (%Z)")'
    2017-09-07 18:00:35 (BST)

(the only difference is in the %Z value - "BST" instead of the incorrect "GMT").

wjlroe avatar Oct 29 '20 21:10 wjlroe

Coverage Status

Coverage increased (+0.2%) to 84.355% when pulling d25af32d80db0573a436b65f63fe5bb146a7abca on wjlroe:fix-dst-strflocaltime into a17dd3248a666d01be75f6b16be37e80e20b0954 on stedolan:master.

coveralls avatar Oct 29 '20 22:10 coveralls

@wtlangford - I cannot say whether this PR fixes all TZ-related bugs in jq, but I hope that you will keep in mind that the serious TZ-related bugs in jq are "silent" and as such really deserve to be prioritized, even over a release of jq 1.7.

If you're too busy to attend to these "silent" TZ-related bugs yourself, perhaps we could recruit some volunteers to review this particular PR, but if we do so, would you be able at least to try to "pull" the PR if the reviewers attest to its being an improvement? Thanks.

pkoppstein avatar Aug 05 '21 19:08 pkoppstein

Issue: #1912.

itchyny avatar Aug 11 '21 11:08 itchyny

Branch 439cf72 works for me, thx.

DehanLUO avatar Jan 08 '23 01:01 DehanLUO

@nicowilliams was this fixed by the recent locale-related patches?

trantor avatar Sep 07 '23 00:09 trantor

@nicowilliams was this fixed by the recent locale-related patches?

No, sadly.

nicowilliams avatar Sep 07 '23 00:09 nicowilliams

@nicowilliams was this fixed by the recent locale-related patches?

No, sadly.

I installed 1.7 yesterday and it seemed to have fixed one of my scripts that uses it? /shrug

chrismo avatar Sep 08 '23 15:09 chrismo