[Intl] fix Locale::getFallback() throwing exception on long $locale #40162
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
8f96c7a to
a41a1c7
Compare
|
Thanks for your contribution, could you please add a testcase to avails a regression in the future? Also it looks like your commit/email is not associated with your github account 🧐 |
|
Friendly ping @ro0NL |
@OskarStark thanks for your review. At first, I added a test case for my change but seems like in CI (appveyor) the Also thank you for informing me about the email. I'll fix it and commit with the right email. |
2469db9 to
57935b9
Compare
|
@OskarStark @ro0NL seems like there is a problem with Doctrine, you can see it here. Please check it, thanks in advance. |
|
The Doctrine failure isn't related. We can ignore it here. |
6f693c8 to
d9d7e8c
Compare
@nicolas-grekas as I wrote here, I have a problem testing my change and I need some help. Can you help me?
|
|
Can you add the test to the PR? |
d9d7e8c to
26ef221
Compare
@nicolas-grekas thank you very much. I added the test successfully. |
| 'LC_MONETARY=fr_FR.UTF-8;LC_MESSAGES=fr_FR.UTF-8;LC_PAPER=fr_FR.UTF-8;LC_NAME=fr_FR.UTF-8;'. | ||
| 'LC_ADDRESS=fr_FR.UTF-8;LC_TELEPHONE=fr_FR.UTF-8;LC_MEASUREMENT=fr_FR.UTF-8;LC_IDENTIFICATION=fr_FR.UTF-8'; | ||
|
|
||
| $this->assertNull(Locale::getFallback($locale)); |
There was a problem hiding this comment.
do we agree root is the fallback then?
compared to above test $this->assertSame('root', Locale::getFallback('nl'));
There was a problem hiding this comment.
actually $defaultFallback / en is expected
There was a problem hiding this comment.
@ro0NL the function is returning null on long $locale by this change:
$localeSubTags = locale_parse($locale) ?? ['language' => $locale];
There was a problem hiding this comment.
I think null is correct also.
05070df to
a89ced8
Compare
|
Thank you @AmirHo3ein13. |
Locale::getFallback()throws an exception when the$localelength is greater thanINTL_MAX_LOCALE_LENso I added a condition to check if locale_parse return null, theLocale::getFallback()don't call\countfunction and just return null instead.