search

azuyalabs / yasumi

The easy PHP Library for calculating holidays
https://www.yasumi.dev
Other
1.05k stars 155 forks source link

Remove locale from Holiday and Provider #122

Closed c960657 closed 4 years ago

c960657 commented 5 years ago

I suggest removing the locale from Holiday and Provider instances. The locale is not used by these entities themselves except when generating a name.

Instead I suggest adding a configurable global locale, and the possibility to override this in calls to Holiday::getName().

This approach is used e.g. by the PHP intl extension, the Symfony Translator, Drupal's t() function and the Punic library.

This will make a clearer separation of concerns as well as reduce the amount of boilerplate code in all the providers.

The change is not backwards compatible, so this is 3.x material.

I have not implemented this for all providers yet. I'd appreciate some feedback on this PR before I proceed with fixing the remaining 9904 failing tests :-)

My reason for suggesting this is to pave the way for another PR that will allow custom locale fallbacks instead of the hardcoded fallback to English. E.g. if you are displaying a Swedish calendar with a Danish locale, not all Swedish holidays may have a Danish translation, but because the two languages are so closely related, you would rather want to fall back to Swedish than English. This change will be much easier if both locale and fallback locales wont have to be passed through all the providers.

stelgenhof commented 5 years ago

Thanks! This is indeed a good suggestion. I like it.

Too bad you just missed the v2 release otherwise this could have been included. I think a v3 release is still far away :)

Anyway let's keep this PR, and as soon as a v3 is around the corner, it can be implemented.

Cheers! Sacha

c960657 commented 5 years ago

It was the release of Yasumi 2.0 that made me look into Yasumi again – hence the bad timing :-)

I have created a new PR, #123, that adds support for a configurable default locale in backwards compatible manner and deprecates passing a locale to Yasumi::create(). The actual removal of $this->holiday from Holiday and Provider will have to wait for Yasumi 3.0.

github-actions[bot] commented 4 years ago

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