-
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 negotiate #9360
base: 4.6
Are you sure you want to change the base?
feat: Strict locale negotiate #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
|
@@ -26,4 +26,10 @@ class Feature extends BaseConfig | |||
* If false, `limit(0)` returns no records. (the behavior of 3.1.9 or later in version 3.x.) | |||
*/ | |||
public bool $limitZeroAsAll = true; | |||
|
|||
/** | |||
* Set `false` to use strict localization comparison (with territory en-*) instead of an abbreviated value. |
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.
* Set `false` to use strict localization comparison (with territory en-*) instead of an abbreviated value. | |
* Set `false` to use strict localization comparison (e.g., against locale code `en-US` instead of the ISO 639-1 language code `en`) |
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.
I will accept michalsn's correction.
|
||
/** | ||
* Set `false` to use strict localization comparison (with territory en-*) instead of an abbreviated value. | ||
* Set `true`, so territory was cut off (en-* as en) before localization comparing. |
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.
* Set `true`, so territory was cut off (en-* as en) before localization comparing. | |
* If set to `true`, the ISO 3166-1 region code will be cut off (e.g., `en-US` as `en`) before localization comparison. |
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.
I will accept michalsn's correction.
@@ -52,4 +52,4 @@ All Changes | |||
This is a list of all files in the **project space** that received changes; | |||
many will be simple comments or formatting that have no effect on the runtime: | |||
|
|||
- @TODO | |||
- @TODO |
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.
what changed here?
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.
I restore code, maybe space?
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.
Description
See #9256
I added a setting for the desired behavior.
Checklist: