Skip to content
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

update LocaleApi and remove old metatdata access #3366

Merged
merged 9 commits into from
Jan 5, 2017
Merged

update LocaleApi and remove old metatdata access #3366

merged 9 commits into from
Jan 5, 2017

Conversation

craigh
Copy link
Member

@craigh craigh commented Jan 3, 2017

in preparation for migration away from ZLanguage and Locale.

Q A
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? no
Fixed tickets -
Refs tickets #3365, #3328, #3180, zikula-modules/Multisites#17
License MIT
Changelog updated yes

Description

update LocaleApi and remove old metatdata access in preparation for migration away from ZLanguage and ZLocale.

Todos

  • Documentation
  • Changelog

@craigh craigh added this to the 1.4.6 milestone Jan 3, 2017
@craigh craigh self-assigned this Jan 3, 2017
@craigh craigh requested a review from Guite January 3, 2017 22:28
@craigh
Copy link
Member Author

craigh commented Jan 4, 2017

wanted opinions on this approach before proceeding with further refactoring.

from which it may be obtained. These variables are not defined explicitly in most cases.


ZLanguage::getLanguageCode() becomes $container->getParameter('locale')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about $request->getLocale()?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw getLocale() below.

What is the difference between ZLanguage::getLanguageCode() and ZLanguage::getLocale()?
What is the difference between $container->getParameter('locale') and $request->getLocale()?

The first is the default and the second one the current request's value only?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose so. not sure.


ZLanguage::getModuleDomain('AcmeFooModule') becomes $kernel->getModule('AcmeFooModule')->getTranslationDomain()

ZLanguage::getDirection() becomes use `dir=auto` in the template instead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{{ localeApi.language_direction }} is deprecated then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not deprecated - removed. broken. see also #3365

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could provide some BC


ZLanguage::getLanguageName($code) becomes \Intl::getLanguageBundle()->getLanguageName($locale)

ZLanguage::getInstalledLanguages() becomes $localeApi->getSupportedLocales()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 😄


ZLanguage::isRequiredLangParam() is handled automatically by the router

ZLanguage::countryMap() becomes \Intl::getRegionBundle()->getCountryNames()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also \Intl::getRegionBundle()->getCountryName($countryCode) available.

@Guite
Copy link
Member

Guite commented Jan 4, 2017

Please also add a pointer to http://symfony.com/doc/current/components/intl.html#accessing-icu-data to the docs.

@Guite
Copy link
Member

Guite commented Jan 4, 2017

I like the direction where this is heading to.

@craigh
Copy link
Member Author

craigh commented Jan 4, 2017

@Guite did you also notice how I changed the scan for locales? It is not using the locale.ini file any longer, but the provided translations. (option 2 from #3328)

@Guite
Copy link
Member

Guite commented Jan 4, 2017

This is fine I think.

@craigh
Copy link
Member Author

craigh commented Jan 5, 2017

@Guite please review again. I think this is ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants