diff options
author | Timothy Flynn <trflynn89@pm.me> | 2023-01-30 12:45:16 -0500 |
---|---|---|
committer | Tim Flynn <trflynn89@pm.me> | 2023-01-30 14:10:07 -0500 |
commit | e74e8381d52813ebb4a5a48e568d4edbae74d559 (patch) | |
tree | 942b819aacf709bc4f24332d47963dc0b5cf8d06 | |
parent | 5c1038e54fcf4a7de0b5b62ca5f99d521c4a12ff (diff) | |
download | serenity-e74e8381d52813ebb4a5a48e568d4edbae74d559.zip |
LibJS: Allow "approximately" results to differ in plural form
This is a normative change in the Intl.NumberFormat V3 spec. See:
https://github.com/tc39/proposal-intl-numberformat-v3/commit/08f599b
Note that this didn't seem to actually affect our implementation. The
Unicode spec states:
https://www.unicode.org/reports/tr35/tr35-53/tr35-numbers.html#Plural_Ranges
"If there is no value for a <start,end> pair, the default result is end"
Therefore, our implementation did not have the behavior noted by the
issue this normative change addressed:
const pr = new Intl.PluralRules("en-US");
pr.selectRange(1, 1); // Is "other", should be "one"
Our implementation already returned "one" here because there is no such
<start=one, end=one> value in the CLDR for en-US. Thus, we already
returned the end value of "one".
6 files changed, 36 insertions, 17 deletions
diff --git a/Userland/Libraries/LibJS/Runtime/Intl/NumberFormat.cpp b/Userland/Libraries/LibJS/Runtime/Intl/NumberFormat.cpp index b31f316497..07e826d538 100644 --- a/Userland/Libraries/LibJS/Runtime/Intl/NumberFormat.cpp +++ b/Userland/Libraries/LibJS/Runtime/Intl/NumberFormat.cpp @@ -1313,7 +1313,7 @@ ThrowCompletionOr<Optional<Variant<StringView, String>>> get_number_format_patte auto formats = ::Locale::get_unit_formats(number_format.data_locale(), number_format.unit(), number_format.unit_display()); auto plurality = MUST_OR_THROW_OOM(resolve_plural(vm, number_format, ::Locale::PluralForm::Cardinal, number.to_value(vm))); - if (auto it = formats.find_if([&](auto& p) { return p.plurality == plurality; }); it != formats.end()) + if (auto it = formats.find_if([&](auto& p) { return p.plurality == plurality.plural_category; }); it != formats.end()) patterns = move(*it); break; @@ -1336,7 +1336,7 @@ ThrowCompletionOr<Optional<Variant<StringView, String>>> get_number_format_patte auto formats = TRY_OR_THROW_OOM(vm, ::Locale::get_compact_number_system_formats(number_format.data_locale(), number_format.numbering_system(), ::Locale::CompactNumberFormatType::CurrencyUnit)); auto plurality = MUST_OR_THROW_OOM(resolve_plural(vm, number_format, ::Locale::PluralForm::Cardinal, number.to_value(vm))); - if (auto it = formats.find_if([&](auto& p) { return p.plurality == plurality; }); it != formats.end()) { + if (auto it = formats.find_if([&](auto& p) { return p.plurality == plurality.plural_category; }); it != formats.end()) { patterns = move(*it); break; } diff --git a/Userland/Libraries/LibJS/Runtime/Intl/PluralRules.cpp b/Userland/Libraries/LibJS/Runtime/Intl/PluralRules.cpp index 8f8527c5cc..ca4e0ce2e9 100644 --- a/Userland/Libraries/LibJS/Runtime/Intl/PluralRules.cpp +++ b/Userland/Libraries/LibJS/Runtime/Intl/PluralRules.cpp @@ -92,13 +92,14 @@ PluralRules::PluralRules(Object& prototype) } // 16.5.3 ResolvePlural ( pluralRules, n ), https://tc39.es/ecma402/#sec-resolveplural -ThrowCompletionOr<::Locale::PluralCategory> resolve_plural(VM& vm, PluralRules const& plural_rules, Value number) +// 1.5.3 ResolvePlural ( pluralRules, n ), https://tc39.es/proposal-intl-numberformat-v3/out/pluralrules/proposed.html#sec-resolveplural +ThrowCompletionOr<ResolvedPlurality> resolve_plural(VM& vm, PluralRules const& plural_rules, Value number) { return resolve_plural(vm, plural_rules, plural_rules.type(), number); } // Non-standard overload of ResolvePlural to allow using the AO without an Intl.PluralRules object. -ThrowCompletionOr<::Locale::PluralCategory> resolve_plural(VM& vm, NumberFormatBase const& number_format, ::Locale::PluralForm type, Value number) +ThrowCompletionOr<ResolvedPlurality> resolve_plural(VM& vm, NumberFormatBase const& number_format, ::Locale::PluralForm type, Value number) { // 1. Assert: Type(pluralRules) is Object. // 2. Assert: pluralRules has an [[InitializedPluralRules]] internal slot. @@ -107,7 +108,7 @@ ThrowCompletionOr<::Locale::PluralCategory> resolve_plural(VM& vm, NumberFormatB // 4. If n is not a finite Number, then if (!number.is_finite_number()) { // a. Return "other". - return ::Locale::PluralCategory::Other; + return ResolvedPlurality { ::Locale::PluralCategory::Other, String {} }; } // 5. Let locale be pluralRules.[[Locale]]. @@ -119,13 +120,16 @@ ThrowCompletionOr<::Locale::PluralCategory> resolve_plural(VM& vm, NumberFormatB auto result = MUST_OR_THROW_OOM(format_numeric_to_string(vm, number_format, number)); // 8. Let s be res.[[FormattedString]]. - auto const& string = result.formatted_string; + auto string = move(result.formatted_string); // 9. Let operands be ! GetOperands(s). auto operands = get_operands(string); - // 10. Return ! PluralRuleSelect(locale, type, n, operands). - return plural_rule_select(locale, type, number, move(operands)); + // 10. Let p be ! PluralRuleSelect(locale, type, n, operands). + auto plural_category = plural_rule_select(locale, type, number, move(operands)); + + // 11. Return the Record { [[PluralCategory]]: p, [[FormattedString]]: s }. + return ResolvedPlurality { plural_category, move(string) }; } // 1.5.4 PluralRuleSelectRange ( locale, type, xp, yp ), https://tc39.es/proposal-intl-numberformat-v3/out/pluralrules/proposed.html#sec-pluralruleselectrange @@ -154,14 +158,20 @@ ThrowCompletionOr<::Locale::PluralCategory> resolve_plural_range(VM& vm, PluralR // 7. Let yp be ! ResolvePlural(pluralRules, y). auto end_plurality = MUST_OR_THROW_OOM(resolve_plural(vm, plural_rules, end)); - // 8. Let locale be pluralRules.[[Locale]]. + // 8. If xp.[[FormattedString]] is yp.[[FormattedString]], then + if (start_plurality.formatted_string == end_plurality.formatted_string) { + // a. Return xp.[[PluralCategory]]. + return start_plurality.plural_category; + } + + // 9. Let locale be pluralRules.[[Locale]]. auto const& locale = plural_rules.locale(); - // 9. Let type be pluralRules.[[Type]]. + // 10. Let type be pluralRules.[[Type]]. auto type = plural_rules.type(); - // 10. Return ! PluralRuleSelectRange(locale, type, xp, yp). - return plural_rule_select_range(locale, type, start_plurality, end_plurality); + // 11. Return ! PluralRuleSelectRange(locale, type, xp.[[PluralCategory]], yp.[[PluralCategory]]). + return plural_rule_select_range(locale, type, start_plurality.plural_category, end_plurality.plural_category); } } diff --git a/Userland/Libraries/LibJS/Runtime/Intl/PluralRules.h b/Userland/Libraries/LibJS/Runtime/Intl/PluralRules.h index 98300b388a..102ce2da36 100644 --- a/Userland/Libraries/LibJS/Runtime/Intl/PluralRules.h +++ b/Userland/Libraries/LibJS/Runtime/Intl/PluralRules.h @@ -6,6 +6,7 @@ #pragma once +#include <AK/String.h> #include <AK/StringView.h> #include <LibJS/Runtime/Completion.h> #include <LibJS/Runtime/Intl/NumberFormat.h> @@ -30,10 +31,15 @@ private: ::Locale::PluralForm m_type { ::Locale::PluralForm::Cardinal }; // [[Type]] }; +struct ResolvedPlurality { + ::Locale::PluralCategory plural_category; // [[PluralCategory]] + String formatted_string; // [[FormattedString]] +}; + ::Locale::PluralOperands get_operands(StringView string); ::Locale::PluralCategory plural_rule_select(StringView locale, ::Locale::PluralForm type, Value number, ::Locale::PluralOperands operands); -ThrowCompletionOr<::Locale::PluralCategory> resolve_plural(VM&, PluralRules const&, Value number); -ThrowCompletionOr<::Locale::PluralCategory> resolve_plural(VM&, NumberFormatBase const& number_format, ::Locale::PluralForm type, Value number); +ThrowCompletionOr<ResolvedPlurality> resolve_plural(VM&, PluralRules const&, Value number); +ThrowCompletionOr<ResolvedPlurality> resolve_plural(VM&, NumberFormatBase const& number_format, ::Locale::PluralForm type, Value number); ::Locale::PluralCategory plural_rule_select_range(StringView locale, ::Locale::PluralForm, ::Locale::PluralCategory start, ::Locale::PluralCategory end); ThrowCompletionOr<::Locale::PluralCategory> resolve_plural_range(VM&, PluralRules const&, Value start, Value end); diff --git a/Userland/Libraries/LibJS/Runtime/Intl/PluralRulesPrototype.cpp b/Userland/Libraries/LibJS/Runtime/Intl/PluralRulesPrototype.cpp index 317f0ecdb5..6b8953a8fa 100644 --- a/Userland/Libraries/LibJS/Runtime/Intl/PluralRulesPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/Intl/PluralRulesPrototype.cpp @@ -36,6 +36,7 @@ ThrowCompletionOr<void> PluralRulesPrototype::initialize(Realm& realm) } // 16.3.3 Intl.PluralRules.prototype.select ( value ), https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.select +// 1.3.3 Intl.PluralRules.prototype.select ( value ), https://tc39.es/proposal-intl-numberformat-v3/out/pluralrules/proposed.html#sec-intl.pluralrules.prototype.select JS_DEFINE_NATIVE_FUNCTION(PluralRulesPrototype::select) { // 1. Let pr be the this value. @@ -45,9 +46,9 @@ JS_DEFINE_NATIVE_FUNCTION(PluralRulesPrototype::select) // 3. Let n be ? ToNumber(value). auto number = TRY(vm.argument(0).to_number(vm)); - // 4. Return ! ResolvePlural(pr, n). + // 4. Return ! ResolvePlural(pr, n).[[PluralCategory]]. auto plurality = MUST_OR_THROW_OOM(resolve_plural(vm, *plural_rules, number)); - return PrimitiveString::create(vm, ::Locale::plural_category_to_string(plurality)); + return PrimitiveString::create(vm, ::Locale::plural_category_to_string(plurality.plural_category)); } // 1.3.4 Intl.PluralRules.prototype.selectRange ( start, end ), https://tc39.es/proposal-intl-numberformat-v3/out/pluralrules/proposed.html#sec-intl.pluralrules.prototype.selectrange diff --git a/Userland/Libraries/LibJS/Runtime/Intl/RelativeTimeFormat.cpp b/Userland/Libraries/LibJS/Runtime/Intl/RelativeTimeFormat.cpp index 0b84d09756..ccf636c947 100644 --- a/Userland/Libraries/LibJS/Runtime/Intl/RelativeTimeFormat.cpp +++ b/Userland/Libraries/LibJS/Runtime/Intl/RelativeTimeFormat.cpp @@ -181,7 +181,7 @@ ThrowCompletionOr<Vector<PatternPartitionWithUnit>> partition_relative_time_patt auto plurality = MUST_OR_THROW_OOM(resolve_plural(vm, relative_time_format.plural_rules(), Value(value))); // 22. Let pattern be po.[[<pr>]]. - auto pattern = patterns.find_if([&](auto& p) { return p.plurality == plurality; }); + auto pattern = patterns.find_if([&](auto& p) { return p.plurality == plurality.plural_category; }); if (pattern == patterns.end()) return Vector<PatternPartitionWithUnit> {}; diff --git a/Userland/Libraries/LibJS/Tests/builtins/Intl/PluralRules/PluralRules.prototype.selectRange.js b/Userland/Libraries/LibJS/Tests/builtins/Intl/PluralRules/PluralRules.prototype.selectRange.js index 716fbd7f5a..76619fafa6 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Intl/PluralRules/PluralRules.prototype.selectRange.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Intl/PluralRules/PluralRules.prototype.selectRange.js @@ -39,11 +39,13 @@ describe("errors", () => { describe("correct behavior", () => { test("basic functionality", () => { const en = new Intl.PluralRules("en"); + expect(en.selectRange(1, 1)).toBe("one"); // one + one = one expect(en.selectRange(1, 2)).toBe("other"); // one + other = other expect(en.selectRange(0, 1)).toBe("other"); // other + one = other expect(en.selectRange(2, 3)).toBe("other"); // other + other = other const pl = new Intl.PluralRules("pl"); + expect(pl.selectRange(1, 1)).toBe("one"); // one + one = one expect(pl.selectRange(1, 2)).toBe("few"); // one + few = few expect(pl.selectRange(1, 5)).toBe("many"); // one + many = many expect(pl.selectRange(1, 3.14)).toBe("other"); // one + other = other |