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

Locale override for getName() (with multiple fallbacks) #195

Merged
merged 5 commits into from
May 1, 2020

Conversation

c960657
Copy link
Contributor

@c960657 c960657 commented Jan 3, 2020

This PR is an alternative to #184. This PR additionally adds support for multiple fallback locales.

Sometimes you need multiple translations for the same holiday.

Pre v2.2.0 you could easily access all translations in $holiday->translations. With the introduction of locale fallback in #176, things got more complicated, especially with substitute holidays (introduced in #162), so accessing the translations property is difficult.

I suggest adding an optional $locales parameter to $holiday->getName(). This allows passing an array of locales to be searched for translations. E.g if ['es_ES', 'de_AT'] is provided, the following lookup priority is used: es_ESesde_ATde.

If no parameter is provided, use the display locale with fallback to DEFAULT_LOCALE and the short name. E.g. if the display locale is es_ES, the following lookup priority is used: es_ESesen_USenshort name.

Note that when providing the optional $locales parameter, DEFAULT_LOCALE and the short name is not used as fallbacks unless explicitly requested. This offers the user better control over fallback behaviour while preserving backwards compatibility.

Parent locales (e.g. de is the parent locale of de_AT) are always used, even when not specified explicitly. Parent locales are not considered a “fallback”. Instead, all strings in a parent locale are considered to be identical in the child locales unless explicitly overridden. I believe this convention is also used elsewhere, e.g. in CLDR.

@stelgenhof
Copy link
Member

Thanks again for the PR! Only had time for a quick look, but seems a good alternative.
Do you prefer this one to be included rather then the previous PR (#184) ?

@c960657
Copy link
Contributor Author

c960657 commented Jan 14, 2020

Yes, this is my favourite :-)

@stelgenhof stelgenhof added this to the v2.3.0 milestone Jan 21, 2020
@stelgenhof
Copy link
Member

@c960657 Planning to do a minor release soonish, so I will review this PR this week.

@github-actions
Copy link

github-actions bot commented Mar 5, 2020

This pull request has been open 30 days with no activity. Please remove the stale label or comment, or this will be closed in 5 days

@stelgenhof stelgenhof requested review from stelgenhof and removed request for stelgenhof May 1, 2020 12:37
@stelgenhof stelgenhof merged commit 91eec58 into azuyalabs:develop May 1, 2020
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