search

twitter / twitter-cldr-rb

Ruby implementation of the ICU (International Components for Unicode) that uses the Common Locale Data Repository to format dates, plurals, and more.
Apache License 2.0
672 stars 93 forks source link

Formatting a full datetime with a time zone gives current time timezone (PST instead of PDT) #228

Closed pierpi2 closed 5 years ago

pierpi2 commented 5 years ago

Hi, the following code:

time = Time.parse("2016-08-29T13:57:49-07:00")
puts time.to_datetime.localize.with_timezone("America/Los_Angeles").to_long_s
time = Time.parse("2016-11-29T12:32:52-08:00")
puts time.to_datetime.localize.with_timezone("America/Los_Angeles").to_long_s

gives:

August 29, 2016 at 1:57:49 PM PST
November 29, 2016 at 12:32:52 PM PST

instead of (using PDT where appropriate):

August 29, 2016 at 1:57:49 PM PDT
November 29, 2016 at 12:32:52 PM PST

Issue i believe is in DateTimeFormatter where current period is always used (if now is PST, PST will be used) timezone_info.current_period.abbreviation.to_s

If I use to_full_s then (as of now, -08:00 is used rather than -07:00, the offset/transitions seem not to be properly handled):

Monday, August 29, 2016 at 1:57:49 PM UTC -08:00
Tuesday, November 29, 2016 at 12:32:52 PM UTC -08:00
camertron commented 5 years ago

Yikes, thanks @pierpi2. I'm surprised this wasn't reported a long time ago. I'd like to release the fix in v5.1.0, which will also contain an upgrade to CLDR v36. Correspondingly I'd be much obliged if you could give the cldr36 branch a try to make sure it fixes this issue for you.

pierpi2 commented 5 years ago

@camertron sorry i just got around testing the branch. It seems the long form is now correct (it produces the PDT where applicable). However, the full form still shows -08:00 as UTC offset even for the August 29, 2016 date. This:

time = Time.parse("2016-08-29T13:57:49-07:00")
puts time.to_datetime.localize.with_timezone("America/Los_Angeles").to_full_s
puts time.to_datetime.localize.with_timezone("America/Los_Angeles").to_long_s

Outputs:

Monday, August 29, 2016 at 1:57:49 PM UTC -08:00
August 29, 2016 at 1:57:49 PM PDT

Not sure if the UTC offset should always refer to the PST time rather than the date it is calculated for.

camertron commented 5 years ago

Oops, thanks @pierpi2 you're right. I just pushed a fix to the cldr36 branch if you'd like to test it out. Should be able to release v5.1.0 next week if things look good.

camertron commented 5 years ago

I just published v5.1.0 with an update to CLDR v36 along with full timezone support, something I've been working on for a while. Hopefully this addresses the timezone issues brought up in this issue.