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
Use new i18n-data component to get country and language name translations #7976
Conversation
47b8dc9
to
702ad9b
Compare
*/ | ||
public function getTranslatedCountry($countryCode, $language=null) | ||
{ | ||
$filePattern = PIWIK_INCLUDE_PATH . '/vendor/piwik/i18n-data/countries/%s.json'; |
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.
I'm just curious. Is it best practice to compose a bath to vendor
like this? Should we maybe have it somewhere in the DI container or is there a method that we can use or so? Maybe it is best practice so just ignore :) Maybe @mnapoli knows more?
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.
We could indeed put the path into the container (like there is path.tmp
, there could be path.vendor
for example). Also maybe in a few years: http://puli.io/
Small comments but otherwise 👍 |
FYI there are already intl data in the internal By the way are we doing anything that could duplicate effort done by any other project? E.g. in the future vision, is there any reason not to want to use something done by Symfony or ZF? Maybe they don't provide translated country names, but for formatting date they probably do? We could maybe get rid of some code. Also regarding the new intl-data component, I downloaded it and it weights 3.2 Mb of JSON files when unzipped. Is this some data that we already have in Piwik today or will some of it not be used? |
Absolutely random but the last thing that popped up just now in my GitHub timeline is this: https://github.com/commerceguys/intl |
Hm. https://github.com/commerceguys/intl seems to hold quite similar data. The reason why the component already has 3.2 MB is because I've added big bunch of translations to languages Piwik currently doesn't have. I'll have some more thoughts on that tomorrow. Maybe there's another (better) solution how to handle those data. |
I'll close this PR for now and issue another one later. I'll integrate the required data within core/Intl and reduce it on that what is realy required |
With this PR the new piwik/i18n-data component is used to get translations for language and country names.
I've extracted those translations to a separate component as it was blowing up our translation files a lot. There will be ~400 translation keys less without those.
The new component is already filled up with a lot of languages piwik is currently not available for. (I've used some other open source libraries holding such data)
To get missing translations for countries and languages I will create a separate transifex project, that will share the translators...
Future vision
After this PR was merged I will check if we also could move the translations for continents, month and day names. Maybe we could also use that component to handle the different date formats. Currently they are handled within the translations which leads to errors in displaying dates in piwik if the format is wrong (which was the case for some languages)