Skip to content
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

Inline GetOperands into PluralRuleSelect #635

Closed
gibson042 opened this issue Jan 6, 2022 · 3 comments · Fixed by #930
Closed

Inline GetOperands into PluralRuleSelect #635

gibson042 opened this issue Jan 6, 2022 · 3 comments · Fixed by #930

Comments

@gibson042
Copy link
Contributor

gibson042 commented Jan 6, 2022

Following up on #594, I'd like to inline GetOperands into PluralRuleSelect, replacing the Record operands parameter of the latter with the String that is currently input of the former ([[FormattedString]] output from FormatNumericToString). The implementation-defined behavior of PluralRuleSelect would still have access to what is now the input of GetOperands and could deconstruct it in that way if/when appropriate.

         1. Let _res_ be ! FormatNumericToString(_pluralRules_, _n_).
-        1. Let _s_ be _res_.[[FormattedString]].
-        1. Let _operands_ be ! GetOperands(_s_).
-        1. Return ! PluralRuleSelect(_locale_, _type_, _n_, _operands_).
+        1. Return ! PluralRuleSelect(_locale_, _type_, _n_, _res_.[[FormattedString]]).

We might also be able to remove the Number n parameter of PluralRuleSelect, but only if implementations are required to select the same plural category for distinct numbers that format identically, such as "one" for 1.01 when rounding options format it as "1" (which I think is the case but would need confirmation):

        1. Return ! PluralRuleSelect(_locale_, _type_, _res_.[[FormattedString]]).
@ben-allen
Copy link
Contributor

I'm reading through these older issues -- do you think this one is still worth pursuing?

@gibson042

@gibson042
Copy link
Contributor Author

Yes. It's weird that ResolvePlural takes in a number, formats it to a string, calls out to a dedicated GetOperands operation (i.e., called from nowhere else) for deconstructing that string and reconstructing a number value for it, and then passes the resulting Record to an opaque implementation-defined PluralRuleSelect operation. Because PluralRuleSelect is opaque, implementations already have the freedom to perform the steps of GetOperands within it as warranted.

@sffc
Copy link
Contributor

sffc commented Aug 22, 2024

ben-allen added a commit to ben-allen/ecma402 that referenced this issue Sep 27, 2024
This AO was only called from the ResolvePlural AO, which immediately passed the Record it returns
into the implementation-defined PluralRuleSelect AO. The revised version directly passes
PluralRuleSelect the decimal String which was previously parsed by GetOperands. Implementations of PluralRuleSelect
may (or may not) use something like GetOperands to determine the plural category returned.

fixes tc39#635
gibson042 pushed a commit that referenced this issue Oct 29, 2024
GetOperands was only called from the ResolvePlural AO, which immediately passed
the returned Record into the implementation-defined PluralRuleSelect AO.
The revised version directly passes PluralRuleSelect the decimal String which
was previously parsed by GetOperands.
Implementations of PluralRuleSelect may (or may not) internally use something
like GetOperands to determine the plural category returned.

fixes #635
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants