[Intl] Fixed support of Locale::getFallback #24157
Conversation
lyrixx
commented
Sep 11, 2017
•
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #24154 |
| License | MIT |
| Doc PR |
|
|
||
| return locale_compose($localesSubTags); | ||
| } | ||
|
|
||
| if (false === $pos = strrpos($locale, '_')) { |
There was a problem hiding this comment.
Can we also make the fallback work with -?
| if (self::$defaultFallback === $locale) { | ||
| return 'root'; | ||
| } | ||
| if (function_exists('locale_parse')) { |
There was a problem hiding this comment.
this looks weird to me. This function is part of ext-intl, so the polyfill will never be used when this function exists
There was a problem hiding this comment.
That's the point. did I miss something?
There was a problem hiding this comment.
ah, I missed the fact that this is not a class from the polyfill. so fine
|
Tests don't appreciate at all this change, see appveyor and travis... :) |
aa7300b to
d419a78
Compare
|
Here we go. The PR is green ;) |
| return 'root'; | ||
| } | ||
| if (function_exists('locale_parse')) { | ||
| $localesSubTags = locale_parse($locale); |
There was a problem hiding this comment.
Should this variable be $localeSubTags instead of $localesSubTags?
| array(null, 'root'), | ||
| ); | ||
|
|
||
| if (function_exists('locale_parse')) { |
There was a problem hiding this comment.
Shouldn't we also add these complex locales (sl_Latn_IT, etc.) to the normal tests when locale_parse() doesn't exist? The result would be of course different ... but I think we should test that too.
|
Note: I don't understand something. PHP seems inconsistent:
edit: Anyway, with this patch, my issue issue is solved in both case (with or without the int function) |
|
👍 |
|
Thank you @lyrixx. |
This PR was merged into the 2.7 branch. Discussion ---------- [Intl] Fixed support of Locale::getFallback | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24154 | License | MIT | Doc PR | Commits ------- 2560552 [Intl] Fixed support of Locale::getFallback
…cales (alanpoulain) This PR was merged into the 3.4 branch. Discussion ---------- [Translation] Use `locale_parse` for computing fallback locales | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | N/A As done in this PR #24157 for the `Intl` component, the `Translation` component should use `locale_parse` as well when available. It will allow to manage [BCP 47](https://tools.ietf.org/html/bcp47) locales, which is why it is considered a bugfix ([locale_set_default](https://www.php.net/manual/en/locale.setdefault.php) is using BCP 47 compliant locale). As done with the forementioned PR, there is also a fallback to make it work with `-`. Sadly, I think it will create some conflicts when merging it upstream since the modified code has changed little by little. Commits ------- 3657c0e Use locale_parse for computing fallback locales