-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Intl] Provide translated timezone names #17636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ce37c78
to
3b3fac3
Compare
0e7bc01
to
b4946ec
Compare
/** | ||
* Returns the location name (usually a city) for a timezone. | ||
* | ||
* @param string $locale The locale to return the name of (e.g. "de_AT"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does not correspond to signature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
b4946ec
to
d74d785
Compare
b9aa2cc
to
16bdf3e
Compare
* | ||
* @return string | ||
*/ | ||
protected function getFallbackName($timezone) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be private
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
e66f0db
to
a2c1257
Compare
@webmozart I agree that the Intl component could be improved or perhaps superseded by one or more new components. It's a bit unclear to me what the purpose of that package is – it provides a copy of a subset of the CLDR datasets (with some opinionated blacklists of pseudo-currencies, pseudo-countries etc.), a partial reimplementation of PHP's ICU DateFormatter for English only, as well as code for parsing ICU data files and converting them to various Symfony-specific file formats. CLDR contains a lot of useful data that I would like to see exposed. Symfony will probably never get support for all the different data, but it should make it easy to fetch the relevant bits from the CLDR data files. I would be interested in working on this, provided that we can agree on a clear direction. I'll add some comments and suggestions to #18368. |
Is the data contained in this PR build based on the same ICU version than the other data already in the component ? If yes, this gets a 👍 from me. If no, we need to recompile the data first. |
Same goes here what I commented in the other PR: the translation quality appears to be sub par and inconsistent, and in that case, no matter the usefulness of the PR itself, we should not want to do it imho. There are several databases that are kept up to date independently that can provide this data. Why burden Symfony with it? |
@jakzal could you have a look at this one please? you're the one doing most of the updates :) |
I don't think it is fair to say that the CLDR translations are poor. CLDR is backed by Microsoft, Google and Apple (among others) who use the data in their software. They all spend considerable ressources on providing high-quality data for a large number of locales. The blame is on me for using the data the wrong way. Since I submitted this PR last year, I have learned a lot more about the CLDR data and the CLDR spec while contributing to the Punic library. The Olson database contains timezone definitions but does not specify how they should be displayed to end users. Symfony simply shows the raw timezone identifier with The Olson database does not designate a name for each timezone apart from the timezone identifier (e.g. A sidenote, not really related to this issue: If you are creating UI that allows users to select a timezone, showing all 400 Olson timezones is probably not the most user-friendly approach. Many of the Olson timezones are duplicates, in the sense that they (in recent years) have identical offsets and DST rules, e.g. So, how does all this apply to Symfony? In order to build a multi-locale appliation that handles dates and timezones, you would usually use a l10n library such as PHPs Intl extension, the Punic library or similar (Symfonys Intl component is not a l10n library in this respect). These packages often provide translated timezone names. E.g. the Intl extension allows you to get the localized city name like this So it seems there is little benefit in adding the timezone names to the Intl package. (I guess the same applies to the other data currently provided by Symfonys Intl component, but The reason I created this PR in the first place was to allow the timezone form component to be localized (PR #17628). Perhaps a better approach is to use PHPs Intl extension directly at run-time when generating the dropdown (with fallback to English if the Intl extension is not installed). What do you think of this? |
@c960657 my apologies if I inferred the quality of your work as a whole was poor, I merely reviewed the proposed changes and concluded that the work being submitted here was not generically usable in its current form. And therefore should not (yet) be a core part of Symfony as it would cause technical debt and BC issues if it were corrected later. |
@curry684 No offense taken :-) It seems we agree that this data is available through other libraries and thus does not belong in Symfony. |
shall we close then? |
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 <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | 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 adds support for translating timezone names. This is useful e.g. in the TimezoneType form type.
The patch only adds support for identifier (Europe/London) to location/city name mapping (London) and vice versa. I assume this is the most common use-case.
The timezone database contains other elements, including meta-zones, alternative names etc. Support for these may be added in the future.