[10.x] Add localization support to the Number::forHumans helper
#49106
Conversation
This has been requested by a large number of people after laravel#48845. My attempt at implementing localization support for this method is using translation helpers. ```php // Example language setup 'sv' => [ 'thousand' => 'tusen', 'million' => 'miljon', 'billion' => 'miljard', 'trillion' => 'biljon', 'quadrillion' => 'biljard', ], ```
caendesilva
force-pushed
the
number-for-humans-helper-localization-support
branch
from
bb85881
to
c3b7ba1
Compare
I just want to preface this with saying that language is complex. Like really complex. It's doubtful that a relatively short and simple method can contain all the edge cases needed for full support of something as complex as this. I totally understand if this is not something that's worth merging, but I wanted to try it out so here it is. We can always revert the commit.
caendesilva
force-pushed
the
number-for-humans-helper-localization-support
branch
from
97b1b0a
to
d08a707
Compare
| $this->assertSame('1,23 miljon', Number::forHumans(1234567, locale: 'sv', precision: 2)); | ||
| } | ||
|
|
||
| public function testToHumansWithPluralizedLocalization() |
There was a problem hiding this comment.
I just want to preface this with saying that language is complex. Like really complex. It's doubtful that a relatively short and simple method can contain all the edge cases needed for full support of something as complex as this. I totally understand if this is not something that's worth merging, but I wanted to try it out so here it is. We can always revert the commit.
| } | ||
|
|
||
| $numberExponent = floor(log10($number)); | ||
| $displayExponent = $numberExponent - ($numberExponent % 3); | ||
| $number /= pow(10, $displayExponent); | ||
|
|
||
| return trim(sprintf('%s %s', static::format($number, $precision, $maxPrecision), $units[$displayExponent] ?? '')); | ||
| $units = [ | ||
| 3 => trans_choice('thousand', $number, locale: $locale ?? static::$locale), |
There was a problem hiding this comment.
Is it right to use global helper methods inside of the framework?
There was a problem hiding this comment.
Good question! I think it's alright as I see global translation helpers being used in Framework code like validation and pagination helpers. But let's keep this open until we get input from someone from the team!
| @@ -141,30 +141,30 @@ public static function fileSize(int|float $bytes, int $precision = 0, ?int $maxP | |||
| * @param int|null $maxPrecision | |||
| * @return string | |||
| */ | |||
| public static function forHumans(int|float $number, int $precision = 0, ?int $maxPrecision = null) | |||
| public static function forHumans(int|float $number, int $precision = 0, ?int $maxPrecision = null, ?string $locale = null) | |||
There was a problem hiding this comment.
Since this feature already released and class not final — this is a breaking change.
There was a problem hiding this comment.
I'm not sure it is. Generally adding parameters to a method signature is not considered breaking in my personal experience with SemVer. Furthermore, while only tangentially related, Laravel does not include named arguments in their backwards compatibility guidelines.
|
Thanks for your pull request to Laravel! Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include. If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions! If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response. |
Abstract
Many people have requested a way to add localization to the
forHumanshelper added in #48845. Here's my attempt at implementing localization support for this method using translation helpers.Example