-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: Strict locale negotiation #9360
feat: Strict locale negotiation #9360
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this may be a solution for some simpler cases, I think more complex ones won't be so simple.
If we take $supportedLocales
as an array of en_US
, fr_FR
, and the browser is set to fr_BE
, the loaded locale will be en_US
. Is this the desired result? We can argue about that.
I think in this case changes would be needed to select the fallback locale.
I'm curious what others think about it.
It current behavior - select first element array |
Yes, I know. While it makes sense when we compare locales loosely. I don't believe it should work that way with strict comparison. Fallback should search for the best match. |
What would be better than the first (main) element of the array? |
In the example I presented it would be We would have to search for the best matching option. It would be |
I tried to add logic to find the nearest locale. But it looks bad. I added a repeat search for Can I save the first variant? |
How about something like this? Although I haven't tested it yet... protected function getBestLocaleMatch(array $supported, ?string $header): string
{
if ($supported === []) {
throw HTTPException::forEmptySupportedNegotiations();
}
if ($header === null || $header === '') {
return $supported[0];
}
$acceptable = $this->parseHeader($header);
$fallbackLocales = [];
foreach ($acceptable as $accept) {
// if acceptable quality is zero, skip it.
if ($accept['q'] === 0.0) {
continue;
}
// if acceptable value is "anything", return the first available
if ($accept['value'] === '*') {
return $supported[0];
}
// look for exact match
if (in_array($accept['value'], $supported, true)) {
return $accept['value'];
}
// set a fallback locale
$fallbackLocales[] = strtok($accept['value'], '-');
}
foreach ($fallbackLocales as $fallbackLocale) {
// look for exact match
if (in_array($fallbackLocale, $supported, true)) {
return $fallbackLocale;
}
// look for locale variants match
foreach ($supported as $locale) {
if (str_starts_with($locale, $fallbackLocale . '-')) {
return $locale;
}
}
}
return $supported[0];
} For |
I did something similar, but right in the |
Oh... I would definitely choose a separate method. Even if some small parts are the same, the main goal is to make it as readable as possible. |
7432d2d
to
29472c6
Compare
Ready. I would pay attention to the description. It is not completely correct. Depends on the array of locales and the query. Sometimes it works, sometimes it returns CodeIgniter4/user_guide_src/source/outgoing/localization.rst Lines 34 to 37 in 046967a
|
Pff.. Again, my small changes overlap with your additions. 😄 |
Yeah, I know - sorry, but I believe this will fit better. I hope I explained my reasons. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready.
i am eagerly wating for this update to go trough :D |
The changes are hanging. It looks like GitHub is fighting against this PR 😅 @neznaika0 can you try to rebase? |
90fe92a
to
159e44d
Compare
Ok. I'm rebasing last commit again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I spotted two things.
c79722d
to
6504d64
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks!
6504d64
to
bddbb95
Compare
Thank you, @neznaika0 |
Description
See #9256
I added a setting for the desired behavior.
Checklist: