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

Bugs in spec regarding currency long names #238

Closed
sffc opened this issue Jun 12, 2018 · 2 comments
Closed

Bugs in spec regarding currency long names #238

sffc opened this issue Jun 12, 2018 · 2 comments
Labels
c: numbers Component: numbers, currency, units s: in progress Status: the issue has an active proposal

Comments

@sffc
Copy link
Contributor

sffc commented Jun 12, 2018

I've been working on spec changes for the Unified NumberFormat proposal. I noticed a couple things about currency long names that didn't look quite right:

Section 11.1.2:

Let stylePatterns be patterns.[[<style>]].
Set numberFormat.[[PositivePattern]] to stylePatterns.[[positivePattern]].
Set numberFormat.[[NegativePattern]] to stylePatterns.[[negativePattern]].

Section 11.1.6:

Let pattern be numberFormat.[[PositivePattern]].
... extract fields out of pattern ...
Else if p is equal "currency" and numberFormat.[[Style]] is "currency", then
..Let currency be numberFormat.[[Currency]].
..Assert: numberFormat.[[CurrencyDisplay]] is "code", "symbol" or "name".
..If numberFormat.[[CurrencyDisplay]] is "code", then
....Let cd be currency.
..Else if numberFormat.[[CurrencyDisplay]] is "symbol", then
....Let cd be an ILD string representing currency in short form. If the implementation does not have such a representation of currency, use currency itself.
..Else if numberFormat.[[CurrencyDisplay]] is "name", then
....Let cd be an ILD string representing currency in long form. If the implementation does not have such a representation of currency, use currency itself.
..Append a new Record { [[Type]]: "currency", [[Value]]: cd } as the last element of result.

The spec makes no distinction between the pattern used for any of the three CurrencyDisplay values; it simply gets the pattern for the style "currency". Then, when substituting into the pattern, it puts whichever currency form in that same spot.

This means that when I change CurrencyDisplay, the only thing that should change, according to the spec, is the form of the currency. The position and everything else should be the same. This is not, of course, what happens:

new Intl.NumberFormat("en", { style: "currency", currency: "EUR" }).format(1.2);
// => "€1.20"
new Intl.NumberFormat("en", { style: "currency", currency: "EUR", currencyDisplay: "code" }).format(1.2);
// => "EUR1.20"
new Intl.NumberFormat("en", { style: "currency", currency: "EUR", currencyDisplay: "name" }).format(1.2);
// => "1.20 euros"
new Intl.NumberFormat("en", { style: "currency", currency: "EUR", currencyDisplay: "name", minimumFractionDigits: 0, maximumFractionDigits: 0 }).format(1);
// => "1 euro"

There are at least three things the spec does not account for:

  1. The position of the currency changes depending on the style.
  2. The form of the currency long name changes based on the plural form.
  3. There may be a space inserted conditionally between the number and the currency. (This is in the CLDR spec and ICU will start doing it for currency codes in ICU 62; it already does it for long names.)

I plan to fix up the spec along with my other changes. Is that okay?

@sffc
Copy link
Contributor Author

sffc commented Jun 15, 2018

@anba

I have the fix drafted in my Unified Number Format proposal. Basically what I did was to make the pattern string in general dependent on more variables, the currency width being one of them; this solves problems 1 and 3. I also changed some language to say that the currency long name may depend on the number being formatted; this solves problem 2.

Full spec change: tc39/proposal-unified-intl-numberformat@e0ee374...sffc:master

See in particular: tc39/proposal-unified-intl-numberformat@cc2ed3d

Thoughts?

@sffc sffc added c: numbers Component: numbers, currency, units s: in progress Status: the issue has an active proposal labels Mar 19, 2019
@sffc
Copy link
Contributor Author

sffc commented Mar 20, 2020

The formatting pattern now depends on CurrencyDisplay since Unified NumberFormat has been checked-in.

@sffc sffc closed this as completed Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: numbers Component: numbers, currency, units s: in progress Status: the issue has an active proposal
Projects
None yet
Development

No branches or pull requests

1 participant