search

elixir-cldr / cldr

Elixir implementation of CLDR/ICU
Other
447 stars 33 forks source link

Allow Cldr.AcceptLanguage to test again locales from Gettext #139

Closed tcitworld closed 3 years ago

tcitworld commented 3 years ago

Cldr.AcceptLanguage considers only locales from cldr (by checking if cldr_locale_name is set) and ignores locales defined from Gettext. This should allow locales from Gettext to be tested against.

kipcole9 commented 3 years ago

Thanks for the PR, much appreciated. Any chance you have a test case? Or an example I can use to build a test case? I will then roll this into the next version I will publish on the weekend.

tcitworld commented 3 years ago

@kipcole9 Sorry for not including tests right away, I was in a hurry and should have taken the time.

kipcole9 commented 3 years ago

No need to apologise at all. I really appreciate the collaboration.

kipcole9 commented 3 years ago

Sorry its taken a while to get to this. I've merged and tests are green so good first step. However I am a bit confused because using your example, the locale oc does not exist in CLDR. So it will be ignored. It so happens that Cldr.AcceptLanguage.best_match/1 will return a valid Cldr.LanguageTag.t but you would have noticed it does not match a CLDR locale and therefore won't be useful for any CLDR-based localisation.

So I'm wondering what use case this helps you with? I'm also wondering if Cldr.AcceptLanguage.best_match/1 should ever return a best match that has no CLDR locale.

kipcole9 commented 3 years ago

The more I think about it the more I think this is not the right strategy. The documentation for Cldr.AcceptLanguage.best_match/1 says:

Parse an Accept-Language string and return the best match for a configured Cldr locale.

Your PR means that contract is no longer true.

Help me understand the use case and I'll happily try to find a good solution but for now I'm going to revert the PR since its a breaking change to the contract of best_match/1.