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

Add test:Intl.NumberFormat using "currency" style and "compact", "engineering", or "scientific" notations. #4274

Merged

Conversation

ben-allen
Copy link
Contributor

@ben-allen ben-allen commented Oct 21, 2024

Related PR: tc39/ecma402#925

fix #4259

@ben-allen ben-allen requested a review from a team as a code owner October 21, 2024 21:51
@ben-allen ben-allen force-pushed the engineering-scientific-currencies-pr-925 branch from 7cf698a to bc4b8f2 Compare October 29, 2024 01:17
Copy link
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

…ncy" style and "compact", "engineering", and "scientific" notations.

Related PR: tc39/ecma402#925
@ptomato ptomato force-pushed the engineering-scientific-currencies-pr-925 branch from bc4b8f2 to 99d6797 Compare October 30, 2024 21:47
@ptomato ptomato merged commit 153db6c into tc39:main Oct 30, 2024
8 checks passed
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Nov 26, 2024
…spidermonkey-reviewers,dminor

Implement the changes from <tc39/ecma402#925>.

Test coverage: <tc39/test262#4274>

Differential Revision: https://phabricator.services.mozilla.com/D228589
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 27, 2024
…spidermonkey-reviewers,dminor

Implement the changes from <tc39/ecma402#925>.

Test coverage: <tc39/test262#4274>

Differential Revision: https://phabricator.services.mozilla.com/D228589
@trflynn89
Copy link
Contributor

Hi @ben-allen - question about this test when the notation is "compact". In that case, wouldn't resolvedOptions.maximumFractionDigits be 0?

In the constructor, we have mnfdDefault=0 and mxfdDefault=3. But when we go into SetNumberFormatDigitOptions, we end up setting needFd to false here:

21. If roundingPriority is "auto", then
    a. Set needSd to hasSd.
    b. If needSd is true, or hasFd is false and notation is "compact", then
       i. Set needFd to false.

hasFd is false (because the test doesn't specify minimumFractionDigits or maximumFractionDigits), and notation is "compact", so needFd becomes false.

needSd is similarly false, so we end up setting both intlObj.[[MinimumFractionDigits]] and intlObj.[[MaximumFractionDigits]] to 0:

24. If needSd is false and needFd is false, then
    a. Set intlObj.[[MinimumFractionDigits]] to 0.
    b. Set intlObj.[[MaximumFractionDigits]] to 0.

@iamstolis
Copy link
Contributor

Hi @ben-allen - question about this test when the notation is "compact". In that case, wouldn't resolvedOptions.maximumFractionDigits be 0?

I agree that maximumFractionDigits should be 0 when notation is "compact". So, I have created the corresponding issue (#4349).

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

Successfully merging this pull request may close these issues.

Need to add test for ECMA402 PR925
4 participants