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

Normative: When formatting currency values, only use data on the number of minor units used to display that currency when using standard notation. #925

Conversation

ben-allen
Copy link
Contributor

@ben-allen ben-allen commented Sep 17, 2024

Previously this data was inappropriately used to set the number of fractional digits displayed when formatting currency values in scientific, engineering, and compact notations. Fix #912.

…er of minor units used to display that currency when using standard notation.

Previously this data was inappropriately used to set the number of fractional digits displayed when formatting currency values in scientific, engineering, and compact notations. See issue tc39#912.
@ben-allen ben-allen marked this pull request as ready for review September 17, 2024 18:56
@ben-allen ben-allen requested a review from gibson042 September 26, 2024 14:01
Copy link
Contributor

@gibson042 gibson042 left a comment

Choose a reason for hiding this comment

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

I love it when the fix is so simple!

@gibson042
Copy link
Contributor

P.S. We should also try to cover this in test262.

@sffc
Copy link
Contributor

sffc commented Oct 8, 2024

(EDIT: wrong issue)

@FrankYFTang
Copy link
Contributor

I believe this is presented to Oct TC39 meeting and still waiting for Mozilla to express their opinion.

@@ -35,6 +35,9 @@ <h1>Intl.NumberFormat ( [ _locales_ [ , _options_ ] ] )</h1>
1. Let _style_ be _numberFormat_.[[Style]].
1. If _style_ is *"currency"*, then
1. Let _currency_ be _numberFormat_.[[Currency]].
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the line " Let currency be numberFormat.[[Currency]]." not inside the same if block of

          1. Let _cDigits_ be CurrencyDigits(_currency_).
          1. Let _mnfdDefault_ be _cDigits_.
          1. Let _mxfdDefault_ be _cDigits_.

Nowhere else need the value of currency in this AO, right?

@FrankYFTang
Copy link
Contributor

@ben-allen ben-allen added has consensus (TG1) Has consensus from TC39-TG1 and removed needs consensus labels Oct 11, 2024
@ben-allen
Copy link
Contributor Author

ben-allen commented Oct 11, 2024

Achieved consensus at October 24 TC39 plenary

@ben-allen ben-allen merged commit 9140da2 into tc39:main Oct 11, 2024
2 checks passed
@FrankYFTang
Copy link
Contributor

tc39/test262#4259

hubot pushed a commit to v8/v8 that referenced this pull request Oct 17, 2024
Spec: tc39/ecma402#925
Bug: 372731679
Change-Id: I7c7bd4974f21c64a3905eb817983636941f410d5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5925080
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Frank Tang <ftang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#96641}
ben-allen added a commit to ben-allen/test262 that referenced this pull request Oct 21, 2024
…ncy" style and "compact", "engineering", and "scientific" notations.

Related PR: tc39/ecma402#925
ben-allen added a commit to ben-allen/test262 that referenced this pull request Oct 29, 2024
…ncy" style and "compact", "engineering", and "scientific" notations.

Related PR: tc39/ecma402#925
ptomato pushed a commit to ben-allen/test262 that referenced this pull request Oct 30, 2024
…ncy" style and "compact", "engineering", and "scientific" notations.

Related PR: tc39/ecma402#925
ptomato pushed a commit to tc39/test262 that referenced this pull request Oct 30, 2024
…ncy" style and "compact", "engineering", and "scientific" notations.

Related PR: tc39/ecma402#925
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus (TG1) Has consensus from TC39-TG1 has consensus Has consensus from TC39-TG2 normative
Projects
None yet
4 participants