-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Intl: Add a new IntlNumberRangeFormatter class #19232
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: master
Are you sure you want to change the base?
Intl: Add a new IntlNumberRangeFormatter class #19232
Conversation
|
||
private function __construct() {} | ||
|
||
public static function createFromSkeleton(string $skeleton, string $locale, int $collapse, int $identityFallback): IntlNumberRangeFormatter {} |
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 hope I won't upset you too much, but I think these changes will need an RFC, because any API changes involve a lot of bikeshedding lately.
For example, we have started to use enums more often (see my https://wiki.php.net/rfc/url_parsing_api RFC), so I think this practice could be continued in your case too.
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.
Hm, I see that you have recently added the ListFormatter class in #18519 that also has some enum-like constants. It probably also makes sense to stay consistent with the current convention of intl (class constants), so I don't insist on my above suggestion.
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.
If we decide that we should employ nicer features in intl, we should do a thorough review of not only the constants ,but other mechanisms (like error handling) as well. And then we can make one compelling holistic RFC (if we would desire to do so ;) ).
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.
Hm, I see that you have recently added the ListFormatter class in #18519 that also has some enum-like constants. It probably also makes sense to stay consistent with the current convention of intl (class constants), so I don't insist on my above suggestion.
Yeah, that was my reasoning. Since that one didn't need an RFC, I thought this one also won't need it.
If we decide that we should employ nicer features in intl, we should do a thorough review of not only the constants ,but other mechanisms (like error handling) as well. And then we can make one compelling holistic RFC (if we would desire to do so ;) ).
Sounds fair. I think for now the API I'm proposing is ok. I do agree introducing the new NumberFormatter will require an RFC.
d55c59b
to
0e083db
Compare
closes #18599
Adds support for ICU's NumberRangeFormatter that returns a locale-aware string interval given 2 numbers. Instead of using the C API, I've opted for the C++ one because it offers a few more possibilities in the future (it could combined with a new NumberFormatter ICU class) while the C version only allows support for skeleton strings.
Since the new NumberFormatter class hasn't been added to PHP Intl yet, the implementation uses skeletons for now.
The implementation uses a factory method that returns a new IntlNumberRangeFormatter object, while the constructor is private. I went with this approach because it would allow the class to have a second factory method in the future for different number formatting (e.g. NumberFormatter). Something like
Drawbacks of the current PHP API:
The C++ version allows to configure the NumberFormatter/skeleton for $start and $end individually (using a fluent interface). The PHP API I propose sets it for both numbers
C++ example code:
and a C example
While the code works, the work is still in progress - I still have to implement a proper error handling. In the meantime, I would like to hear your opinions about the PHP interface I'm proposing.