-
-
Notifications
You must be signed in to change notification settings - Fork 145
[Intl] Add PHP 8.5 IntlListFormatter
as installable polyfill
#532
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
base: 1.x
Are you sure you want to change the base?
Conversation
fa3be1e
to
fa352fc
Compare
Adds a new polyfill component `symfony/polyfill-intl-listformatter` that provides the functionality of the new `IntlListFormatter` to PHP 7.2 and later. It is a new composer-installable package (similar to how Intl `MessageFormatter` is a separate installable package, and the PHP 8.5 polyfill can require it as a dependency too. It only supports `en` locale. However, PHP applications that require this package can extend the namespaced polyfill (`\Symfony\Polyfill\Intl\ListFormatter\IntlListFormatter`) and add their own list patterns by extending the static variable (`\Symfony\Polyfill\Intl\ListFormatter\IntlListFormatter::$listPatterns`) that holds these patterns. - [ICU listPatterns](https://github.com/unicode-org/cldr-json/blob/main/cldr-json/cldr-misc-full/main/en/listPatterns.json) - [php-src commit](php/php-src@3f7545245) - [PHP.Watch: IntlListFormatter](https://php.watch/versions/8.5/IntlListFormatter) Closes symfonyGH-530.
fa352fc
to
3b446ce
Compare
// cc @BogdanUngureanu, I could use your inputs/review here :) |
* | ||
* @internal | ||
*/ | ||
class IntlListFormatter |
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.
It should be final, non-serializable and not able to clone it.
]); | ||
} | ||
|
||
protected function getListPattern(): array { |
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.
Why not private?
private $width; | ||
|
||
protected static $listPatterns = [ | ||
'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.
Maybe make it clear that this is en_US since it's using the Oxford comma?
) { | ||
$exceptionClass = PHP_VERSION_ID >= 80000 ? \ValueError::class : \InvalidArgumentException::class; | ||
if ($locale !== 'en' && strpos($locale, 'en') !== 0) { | ||
throw new $exceptionClass('Invalid locale, only "en" and "en-*" locales are supported'); |
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 have some mixed feelings about this. Since it's using the en-US set, for en-gb it would return a different result than the real class.
This should be added in symfony/polyfill-intl-icu (with the other formatters) instead of being a new package IMO
Extending the internal class is not covered by BC and is not a supported use case. |
Adds a new polyfill component
symfony/polyfill-intl-listformatter
that provides the functionality of the newIntlListFormatter
to PHP 7.2 and later.It is a new composer-installable package (similar to how Intl
MessageFormatter
is a separate installable package, and the PHP 8.5 polyfill can require it as a dependency too.It only supports
en
locale. However, PHP applications that require this package can extend the namespaced polyfill(
\Symfony\Polyfill\Intl\ListFormatter\IntlListFormatter
) and add their own list patterns by extending the static variable (\Symfony\Polyfill\Intl\ListFormatter\IntlListFormatter::$listPatterns
) that holds these patterns.Closes GH-530.