[Intl] Add Timezones #28831

ro0NL · 2018-10-12T09:29:08Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	replaces #26851, #17628, #17636
License	MIT
Doc PR	symfony/symfony-docs#11447

This PR compiles a new TimezoneBundle from ICU data, based on the following resources:

The goal is to provide a fixed set of timezones, with a localized human readable name.

For inspiration and to double check some data i used the timezone widget from google, e.g. when using Google Calendar.

I've only pushed some common compiled locales for review, the rest will follow once finished.

cc @jakzal

ro0NL · 2018-10-12T10:09:10Z

note im not yet convinved about form type integration... if we go that way, we might want an option intl => true to optionally change the source from DateTimeZone::listIdentifiers() to TimezoneBundle

The intl variant IMHO should include, and be sorted by offset, from -xx:00 to +xx:00.

here we also see a different format meta - city vs. meta (city), i tend to prefer the latter =/

This PR was merged into the 4.2-dev branch. Discussion ---------- [Form] Deprecate TimezoneType regions option | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no  | Deprecations? | yes | Tests pass? | yes  | Fixed tickets | #28848 | License | MIT | Doc PR | symfony/symfony-docs#...  I know i've added this option myself in 4.1, but given my recent development for #28624 i realized it's an opinionated feaure, which can/should be solved on user-side (`choice_filter/choice_loader` and/or `group_by`). - blocks translations as we dont have them (see #28831) - blocks possibility of switching to Intl zones which doesnt really have this filter feature (see #28836) ~While at it, i solved a few issues with `OptionsResolver` that is able to deprecate options as of 4.2 also.~ Fixed in #28878 - when resolved trigger the deprecation - allow to opt-out from triggering the deprecation - dont trigger deprecation for default values (only given ones) Commits ------- 5cb532d [Form] Deprecate TimezoneType regions option

src/Symfony/Component/Intl/CHANGELOG.md

chalasr

👍 Once PR rebased and remaining comment addressed

ro0NL · 2018-12-13T16:42:17Z

ill rebase and compile the full list tomorrow. Meanwhile, can we decide on #28846 Not sure we want to add the boilerplate classes now, only to deprecate them later again.

ro0NL · 2018-12-13T16:46:13Z

src/Symfony/Component/Intl/Data/Generator/ZoneDataGenerator.php

+            }
+
+            if (null === $city && 0 !== strrpos($zone, 'Etc:') && false !== $i = strrpos($zone, ':')) {
+                $city = str_replace('_', ' ', substr($zone, $i + 1));


i need to check if this fallback is still needed. Ive added the root bundle fallback above later, and AFAIK it makes this one redundant.

status: needs work

for future reference, this solves

- "Africa\/Abidjan": "Greenwich Mean Time (Abidjan)", - "Africa\/Accra": "Greenwich Mean Time (Accra)", - "Africa\/Addis_Ababa": "East Africa Time (Addis Ababa)", - "Africa\/Algiers": "Central European Standard Time (Algiers)", + "Africa\/Abidjan": "Greenwich Mean Time", + "Africa\/Accra": "Greenwich Mean Time", + "Africa\/Addis_Ababa": "East Africa Time", + "Africa\/Algiers": "Central European Standard Time",

i think it's more explicit to always include the example city (ec), and i believe for these cases when it's not provided by intl we can infer it from here.

src/Symfony/Component/Intl/Data/Generator/ZoneDataGenerator.php

nicolas-grekas · 2019-03-13T15:53:19Z

@ro0NL up to rebase and address any remaining comment?

ro0NL · 2019-03-16T08:54:07Z

@nicolas-grekas i've refigured out how the compile process went, and added a binary for it as such. Works for me :) Please validate.

Need to solve remaining comments still, but compiling is fast at least now :)

src/Symfony/Component/Intl/.gitignore

This PR was merged into the 3.4 branch. Discussion ---------- [Intl] Add compile binary | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no-ish | BC breaks? | no  | Deprecations? | no | Tests pass? | yes  | Fixed tickets | #...  | License | MIT | Doc PR | symfony/symfony-docs#...  Compile the Intl data by invoking a single command, and make it work out-of-the-box. (Split from #28831) ```bash $ src/Symfony/Component/Intl/Resources/bin/compile ``` run in repository root because of https://github.com/symfony/symfony/blob/b7e798ef745a09ca2e76fba4afad0a04fcbd9195/src/Symfony/Component/Intl/Data/Generator/LocaleDataGenerator.php#L141 3.4 is ok, 4.2 is not because of #28833 but new locales are introduced in https://github.com/symfony/symfony/pull/28977/files#diff-f52da32e1ee6b93598814090d0749aa6R1 So as long as 3.4 is supported, but branches above add filters etc. during generation we're risking this discrepancy. I suggest after merge in upper branches to re-run `compile` (potential for automating, but run if needed :)) Commits ------- 426b92f [Intl] Add compile binary

webmozart · 2019-04-06T15:17:25Z

Should be rebased and finished as soon as #28846 is merged IMO.

src/Symfony/Component/Intl/Data/Provider/TimezoneDataProvider.php

This PR was squashed before being merged into the 4.3-dev branch (closes #28846). Discussion ---------- [Intl] Simplify API | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no  | Deprecations? | no | Tests pass? | yes  | Fixed tickets | #18368 | License | MIT | Doc PR | symfony/symfony-docs#11221 Simplifies the Intl API. It greatly reduces the no. of boilerplate classes in this component. Very over complicated, much wow :) Solving (IMHO): ```php class LanguageBundle extends LanguageDataProvider implements LanguageBundleInterface ``` Which seems very over complicated just to provide static data. ```php // before Intl::getLanguageBundle()->getLanguageName() // string | null // after Languages::getName() // string Languages::exists() // bool ``` I left out Canonicalization on puropose, that's a new topic to me. - [x] Languages - [x] Locales - [x] Currencies - [x] Regions - [x] Scripts - [ ] Timezones (#28831) - [x] Update constraints - [x] Update form types Thoughts? Commits ------- d6b67d4 [Intl] Simplify API

fabpot · 2019-04-15T11:54:37Z

@ro0NL #28846 has been merged now, so I think we can work on finishing this one.

ro0NL · 2019-04-16T20:00:01Z

Hm right, i felt awkward about a possible double -, yet Google did this:

As a plain list, without any context, i chose to avoid this duplication by using ().

For the form, an alternative approach is to use optgroups per offset. Semantically (for JS & co) that might be more useful even. Havent come to play around with this though.

ro0NL · 2019-04-17T07:49:44Z

compiled the TZ offsets :)

Hanmac · 2019-04-17T09:11:06Z

hm shouldn't the offset be based on DST too?
Europe/Berlin as MEZ (+01:00) and MESZ (+02:00)

ro0NL · 2019-04-17T09:31:08Z

hm yes :) but we need a local time for that. Something like https://3v4l.org/57U2J could be taken into account in get(Raw)Offset(), allowing a local date (default now) to be specified.

status: needs work

ro0NL · 2019-04-17T10:13:18Z

@Hanmac i've removed the last commit, i dont think we need to compile offsets and instead simply use DateTimeZone directly.

status: needs review (still ready :))

Hanmac · 2019-04-17T14:35:26Z

@ro0NL Question about the Translation: i saw in the german translation something about normal time, but isn't that wrong for DST and should it say summer time instead?

maybe instead of going directly from DateTimeZone "Europe/Berlin" to Translation, maybe instead use getTransitions for the abbr like "CET"/"CEST" and translate that ?

https://github.com/unicode-org/icu/blob/master/icu4c/source/data/zone also has different lables for that

ro0NL · 2019-04-17T18:59:20Z

@Hanmac i checked the google widget again :) today it includes

(GMT+02:00) Midden-Europese tijd - Amsterdam

(in english Central European Time)

so per https://github.com/unicode-org/icu/blob/78a4abc5ed5db9ddadb57dca0f5222e7639c124c/icu4c/source/data/zone/en.txt#L292-L295 it seems to always pick lg (out of lg, ls and ld) and only differs the offset based on local time.

I interpreted these codes as

lg: long generic, e.g. "Central European Time"
ls: long specific (not DST), e.g. "Central European Standard Time"
ld: long DST, e.g. "Central European Summer Time"

(http://site.icu-project.org/design/formatting/timezone/icu-4-8-time-zone-names#TOC-UTS-35-Time-Zone-Display-Name-Basics)

Symfony now follows ls > lg > ld. I tend to believe we should also always use lg instead, as the fallback probably never happens (not sure why i picked ls instead 🤔 perhaps due this "standard" term)

So using lg gives in fact shorter names 🎉 and is more in line with what @javiereguiluz expected in #28831 (comment) (AFAIK there's no mapping, nor translations available for continents, e.g. "Europe")

IMHO using the generic name solves the issue of names varied based on local time, thus a more simple implementation 👍 (the offset should be dynamic and is a display/rendering concern for now - unrelated here)

status: needs work

ro0NL · 2019-04-17T19:28:46Z

@Hanmac see 3e09816, can you verify ... IMHO the results are much better now.

status: needs review

cc @fabpot

Hanmac · 2019-04-17T20:01:59Z

the Translations are in the Meta like de used for CET or MEZ meta:Europe_Central like lg{"Mitteleuropäische Zeit"} if you want you could use sg{"MEZ"} too.
From what i have seen you already use the meta stuff?

i think it is okay for now if the name you show is DST independent.

I think what the pro way would be:

input timezone String "Europe/Berlin" call getTransitions on it with now time
that gives you the offset and the abbr "CET"/"CEST" and if DST is on or off

do you need to parse the abbr?
i got the thinking you might need https://github.com/unicode-org/icu/blob/master/icu4c/source/data/zone/tzdbNames.txt but i am unsure now, because you already knew from the DST param what kind of label it should use? But that would make it depend on the local time and you can't prebuild it?

i currently don't know what the best way would be there

ro0NL · 2019-04-17T20:17:41Z

@Hanmac can you check once more :) the fallback for ls is in fact required for translating Etc/GMT & co 🤷‍♂️

de.json currently is "Europe\/Berlin": "Mitteleuropäische Zeit (Berlin)", that's OK right? Im not sure i understand why we need the abbr. :/

From what i have seen you already use the meta stuff?

Yes

https://github.com/unicode-org/icu/blob/30d203459720bb64c850680a79b0f393a60934ca/icu4c/source/data/misc/metaZones.txt#L3553-L3557 gives us europe/berlin > Europe_Central

https://github.com/unicode-org/icu/blob/30d203459720bb64c850680a79b0f393a60934ca/icu4c/source/data/zone/de.txt#L1512-L1514 gives us Europe_Central > lg (Mitteleuropäische Zeit)

i think it is okay for now if the name you show is DST independent.

agree :)

I think what the pro way would be:

input timezone String "Europe/Berlin" call getTransitions on it with now time

that gives you the offset and the abbr "CET"/"CEST" and if DST is on or off

agree :)

do you need to parse the abbr?

again, not sure how the abbr. is related...

ro0NL · 2019-04-17T20:22:08Z

to clarify, Symfony Timezones will not provide you the DST specific name (for now), only the generic one. Hence i suggested to only show the local offset with the generic tz name; that's what google does.

ro0NL · 2019-04-17T20:27:08Z

I think i understand, sg, ss, sd, most likely stands for "short generic/standard/dst" 😅 didnt know these were translatable (CEST in dutch, is MESZ in german :|)

Still, for the name itself, it's just another fallback. We only provide the long generic name for now.

fabpot · 2019-04-21T18:40:52Z

Thank you @ro0NL.

@jakzal

This PR was squashed before being merged into the 4.3-dev branch (closes #28831). Discussion ---------- [Intl] Add Timezones | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no  | Deprecations? | no | Tests pass? | yes  | Fixed tickets | replaces #26851, #17628, #17636 | License | MIT | Doc PR | symfony/symfony-docs#11447 This PR compiles a new `TimezoneBundle` from ICU data, based on the following resources: - https://github.com/unicode-org/icu/tree/master/icu4c/source/data/zone - https://github.com/unicode-org/icu/blob/master/icu4c/source/data/misc/timezoneTypes.txt - https://github.com/unicode-org/icu/blob/master/icu4c/source/data/misc/metaZones.txt The goal is to provide a fixed set of timezones, with a localized human readable name. For inspiration and to double check some data i used the timezone widget from google, e.g. when using Google Calendar. I've only pushed some common compiled locales for review, the rest will follow once finished. cc @jakzal Commits ------- 4bea198 [Intl] Add Timezones

This PR was merged into the 4.3-dev branch. Discussion ---------- [Validator] Allow intl timezones | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no  | Deprecations? | no | Tests pass? | yes  | Fixed tickets | ref #28836 | License | MIT | Doc PR | symfony/symfony-docs#11502 This PR considers the ICU timezones (#28831) as valid, as well as the PHP ones. Both can be used in `DateTimeZone`, but expired timezones cannot be used with `IntlTimeZone`. The `intlCompatibility` option ensures compatibility between both. Practically this allows for both `UTC` and `Etc/UTC`, where the latter should be preferred. I.e. currently `Etc/UTC` is considered an invalid timezone ... which it's not. Commits ------- 7294b59 [Validator] Allow intl timezones

carsonbot added Status: Needs Review Intl Feature labels Oct 12, 2018

ro0NL changed the title ~~[Intl][WIP] Add TimezoneBundle~~ [Intl] Add TimezoneBundle Oct 12, 2018

This was referenced Oct 12, 2018

[Form] Add time zone translations #26851

Closed

[RFC][Form] Change TimezoneType to Intl sources / translate timezone choice labels #28836

Closed

[Intl] Simplify API #28846

Merged

[Form] Deprecate TimezoneType regions option #28860

Merged

nicolas-grekas added this to the next milestone Oct 14, 2018

nicolas-grekas approved these changes Dec 1, 2018

View reviewed changes

src/Symfony/Component/Intl/CHANGELOG.md Show resolved Hide resolved

carsonbot added Status: Reviewed and removed Status: Needs Review labels Dec 1, 2018

chalasr reviewed Dec 12, 2018

View reviewed changes

ro0NL commented Dec 13, 2018

View reviewed changes

src/Symfony/Component/Intl/Data/Generator/ZoneDataGenerator.php Outdated Show resolved Hide resolved

ro0NL commented Dec 13, 2018

View reviewed changes

src/Symfony/Component/Intl/Data/Generator/ZoneDataGenerator.php Outdated Show resolved Hide resolved

ro0NL mentioned this pull request Mar 16, 2019

[Intl] Add compile binary #30584

Merged

nicolas-grekas reviewed Mar 16, 2019

View reviewed changes

src/Symfony/Component/Intl/.gitignore Outdated Show resolved Hide resolved

nicolas-grekas added the Ready label Mar 16, 2019

nicolas-grekas removed the Ready label Mar 20, 2019

stof reviewed Apr 7, 2019

View reviewed changes

src/Symfony/Component/Intl/Data/Provider/TimezoneDataProvider.php Outdated Show resolved Hide resolved

carsonbot added Status: Needs Work and removed Status: Reviewed labels Apr 17, 2019

ro0NL mentioned this pull request Apr 17, 2019

[Intl] Add Timezones symfony/symfony-docs#11447

Closed

carsonbot added Status: Needs Review and removed Status: Needs Work labels Apr 17, 2019

fabpot approved these changes Apr 21, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Apr 21, 2019

[Intl] Add Timezones

4bea198

fabpot merged commit 4bea198 into symfony:master Apr 21, 2019

ro0NL deleted the timezone branch April 22, 2019 07:04

ro0NL mentioned this pull request Apr 27, 2019

[Validator] Allow intl timezones #31292

Merged

nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

fabpot mentioned this pull request May 9, 2019

Release v4.3.0-BETA1 #31435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Intl] Add Timezones #28831

[Intl] Add Timezones #28831

ro0NL commented Oct 12, 2018 •

edited

Loading

ro0NL commented Oct 12, 2018 •

edited

Loading

chalasr left a comment •

edited

Loading

ro0NL commented Dec 13, 2018

ro0NL Dec 13, 2018

ro0NL Mar 16, 2019 •

edited

Loading

nicolas-grekas commented Mar 13, 2019

ro0NL commented Mar 16, 2019

webmozart commented Apr 6, 2019

fabpot commented Apr 15, 2019

ro0NL commented Apr 16, 2019

ro0NL commented Apr 17, 2019

Hanmac commented Apr 17, 2019

ro0NL commented Apr 17, 2019

ro0NL commented Apr 17, 2019 •

edited

Loading

Hanmac commented Apr 17, 2019 •

edited

Loading

ro0NL commented Apr 17, 2019 •

edited

Loading

ro0NL commented Apr 17, 2019

Hanmac commented Apr 17, 2019

ro0NL commented Apr 17, 2019 •

edited

Loading

ro0NL commented Apr 17, 2019 •

edited

Loading

ro0NL commented Apr 17, 2019

fabpot commented Apr 21, 2019

[Intl] Add Timezones #28831

[Intl] Add Timezones #28831

Conversation

ro0NL commented Oct 12, 2018 • edited Loading

ro0NL commented Oct 12, 2018 • edited Loading

chalasr left a comment • edited Loading

Choose a reason for hiding this comment

ro0NL commented Dec 13, 2018

ro0NL Dec 13, 2018

Choose a reason for hiding this comment

ro0NL Mar 16, 2019 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas commented Mar 13, 2019

ro0NL commented Mar 16, 2019

webmozart commented Apr 6, 2019

fabpot commented Apr 15, 2019

ro0NL commented Apr 16, 2019

ro0NL commented Apr 17, 2019

Hanmac commented Apr 17, 2019

ro0NL commented Apr 17, 2019

ro0NL commented Apr 17, 2019 • edited Loading

Hanmac commented Apr 17, 2019 • edited Loading

ro0NL commented Apr 17, 2019 • edited Loading

ro0NL commented Apr 17, 2019

Hanmac commented Apr 17, 2019

ro0NL commented Apr 17, 2019 • edited Loading

ro0NL commented Apr 17, 2019 • edited Loading

ro0NL commented Apr 17, 2019

fabpot commented Apr 21, 2019

ro0NL commented Oct 12, 2018 •

edited

Loading

ro0NL commented Oct 12, 2018 •

edited

Loading

chalasr left a comment •

edited

Loading

ro0NL Mar 16, 2019 •

edited

Loading

ro0NL commented Apr 17, 2019 •

edited

Loading

Hanmac commented Apr 17, 2019 •

edited

Loading

ro0NL commented Apr 17, 2019 •

edited

Loading

ro0NL commented Apr 17, 2019 •

edited

Loading

ro0NL commented Apr 17, 2019 •

edited

Loading