Normative: Integrate Intl NumberFormat v3 into ECMA-402 #753

sffc · 2023-01-30T17:11:45Z

https://github.com/tc39/proposal-intl-numberformat-v3

sffc · 2023-01-30T17:31:17Z

@gibson042 or @ryzokuken can you suggest how to fix the following ecmarkup error:

          1. Let _n1_ and _e1_ each be an integer and _r1_ a mathematical value, with <emu-eqn>_r1_ = ToRawPrecisionFn(_n1_, _e1_, _p_)</emu-eqn>, such that <emu-eqn>_r1_ ≤ _x_</emu-eqn> and _r1_ is maximized.
...
          1. Let _r_ be ApplyUnsignedRoundingMode(_x_, _r1_, _r2_, _unsignedRoundingMode_).

causes errors such as

[2023-01-30T17:13:34.352Z] Error: spec/numberformat.html:1263:56: could not find a preceding declaration for "r1"
[2023-01-30T17:13:34.352Z] Error: spec/numberformat.html:1263:62: could not find a preceding declaration for "r2"

gibson042 · 2023-01-30T19:14:45Z

@sffc https://tc39.es/ecmarkup/#variable-declarations

-          1. Let _n1_ and _e1_ each be an integer and _r1_ a mathematical value, with <emu-eqn>_r1_ = ToRawPrecisionFn(_n1_, _e1_, _p_)</emu-eqn>, such that <emu-eqn>_r1_ ≤ _x_</emu-eqn> and _r1_ is maximized.
-          1. Let _n2_ and _e2_ each be an integer and _r2_ a mathematical value, with <emu-eqn>_r2_ = ToRawPrecisionFn(_n2_, _e2_, _p_)</emu-eqn>, such that <emu-eqn>_r2_ ≥ _x_</emu-eqn> and _r2_ is minimized.
+          1. [declared="n1,e1,r1"] Let _n1_ and _e1_ each be an integer and _r1_ a mathematical value, with <emu-eqn>_r1_ = ToRawPrecisionFn(_n1_, _e1_, _p_)</emu-eqn>, such that <emu-eqn>_r1_ ≤ _x_</emu-eqn> and _r1_ is maximized.
+          1. [declared="n2,e2,r2"] Let _n2_ and _e2_ each be an integer and _r2_ a mathematical value, with <emu-eqn>_r2_ = ToRawPrecisionFn(_n2_, _e2_, _p_)</emu-eqn>, such that <emu-eqn>_r2_ ≥ _x_</emu-eqn> and _r2_ is minimized.

sffc · 2023-01-30T19:22:52Z

Reviewers: please double-check that the diff in the PR corresponds to the diff in the proposal. I will also double-check this but it's good to have extra verification of that.

gibson042

I still need to review the bulk of this PR in numberformat.html, but I have some comments on the changes that touch pluralrules.html.

spec/pluralrules.html

ryzokuken

Overall the spec looks great, I left a few comments inline but most of them are minor and some could even be addressed later on.

spec/negotiation.html

ryzokuken · 2023-04-04T14:20:41Z

spec/numberformat.html

+    </emu-clause>
+
+    <emu-clause id="sec-formatnumericrangetoparts" aoid="FormatNumericRangeToParts">
+      <h1>FormatNumericRangeToParts ( _numberFormat_, _x_, _y_ )</h1>


Can we switch to the structured headers syntax? It doesn't necessarily need to be done before this is merged, but let's do it sooner rather than later.

This applies to every new AO, I suppose.

Acknowledged. I wrote most of this spec before structured headers were common in ECMA-402. In an effort to get this merged, I would rather make only small changes that are unlikely to require extensive further review. However, this sounds like a great editorial task to be done after merging this PR. :)

ryzokuken · 2023-04-04T14:23:11Z

spec/numberformat.html

-        <dd>It populates the internal slots of _intlObj_ that affect locale-independent number rounding (see <emu-xref href="#sec-formatnumberstring"></emu-xref>).</dd>
-      </dl>
+    <emu-clause id="sec-setnfdigitoptions" aoid="SetNumberFormatDigitOptions">
+      <h1>SetNumberFormatDigitOptions ( _intlObj_, _options_, _mnfdDefault_, _mxfdDefault_, _notation_ )</h1>


Is there an important reason to move away from the structured headers syntax here? Since the new description doesn't include the "type" information unlike the previous style, we're just losing that information...

Agreed, it looks like this change should be reverted to keep the current SetNumberFormatDigitOptions <h1> and <dl>.

May have gotten lost as part of the merge. Will fix.

ryzokuken · 2023-04-04T14:26:01Z

spec/numberformat.html

+        1. If _roundingIncrement_ is not in « 1, 2, 5, 10, 20, 25, 50, 100, 200, 250, 500, 1000, 2000, 2500, 5000 », throw a *RangeError* exception.
+        1. Let _roundingMode_ be ? GetOption(_options_, *"roundingMode"*, ~string~, &laquo; *"ceil"*, *"floor"*, *"expand"*, *"trunc"*, *"halfCeil"*, *"halfFloor"*, *"halfExpand"*, *"halfTrunc"*, *"halfEven"* &raquo;, *"halfExpand"*).
+        1. Let _trailingZeroDisplay_ be ? GetOption(_options_, *"trailingZeroDisplay"*, ~string~, &laquo; *"auto"*, *"stripIfInteger"* &raquo;, *"auto"*).
+        1. NOTE: All fields required by SetNumberFormatDigitOptions have now been read from _options_. The remainder of this AO interprets the options and may throw exceptions.


Don't feel strongly either way, but not entirely convinced that this NOTE is necessary. That said, if it helps implementers enough to justify keeping it in, why not

I added this line as part of resolving an editorial issue back on the spec repository where I was reading options in a bunch of different places. I think the NOTE could be useful for future changes in order to ensure that we are consistent in the point at which we read the options.

spec/numberformat.html

ryzokuken · 2023-04-04T14:40:47Z

spec/numberformat.html

+    <emu-clause id="sec-collapsenumberrange" aoid="CollapseNumberRange">
+      <h1>CollapseNumberRange ( _result_ )</h1>
+      <p>
+        The CollapseNumberRange abstract operation modifies _result_, which must be a List of Record values as described in PartitionNumberRangePattern, by removing redundant information, and returns the resulting List. The algorithm is implementation dependent, but it must not introduce ambiguity: the resulting list after modification should be unique for all pairs _x_ and _y_.


It's difficult to understand this sentence without context, since one might not know x and y, could it be worded differently? Like I think "for all pairs start and end" already sounds easier to wrap your head around.

ryzokuken · 2023-04-04T14:42:19Z

spec/numberformat.html

+        The FormatApproximately abstract operation modifies _result_, which must be a List of Record values as described in PartitionNumberPattern, by adding a new Record for the approximately sign, which may depend on _numberFormat_ (which must be an object initialized as NumberFormat). The following steps are taken:
+      </p>
+      <emu-alg>
+        1. Let _i_ be an index into _result_, determined by an implementation-defined algorithm based on _numberFormat_ and _result_.


nit: could we add a bit more info to guide implementations here at all?

Added a sentence

spec/numberformat.html

gibson042

I agree with @ryzokuken , and have a few nits/suggestions but everything overall looks good.

spec/numberformat.html

gibson042 · 2023-04-04T19:54:38Z

spec/numberformat.html

        1. Else,
-          1. Set _intlObj_.[[RoundingType]] to ~fractionDigits~.
+          1. Set _intlObj_.[[RoundingType]] to ~morePrecision~.


I think these steps can also be reordered for clarity by setting [[RoundingType]] only after every step that sets [[{Minimum,Maximum}{Fraction,Significant}Digits]]:

1. If _needSd_ is *true*, then …set _intlObj_.[[{Minimum,Maximum}SignificantDigits]] based on {_mnsd_,_mxsd_} or {1,21}… 1. If _needfd_ is *true*, then …set _intlObj_.[[{Minimum,Maximum}FractionDigits]] based on {_mnfd_,_mxfd_} or {_mnfdDefault_,_mxfdDefault_}… 1. If _needSd_ is *false* and _needFd_ is *false*, then 1. Set _intlObj_.[[MinimumFractionDigits]] to 0. 1. Set _intlObj_.[[MaximumFractionDigits]] to 0. 1. Set _intlObj_.[[MinimumSignificantDigits]] to 1. 1. Set _intlObj_.[[MaximumSignificantDigits]] to 2. 1. Set _intlObj_.[[RoundingType]] to ~morePrecision~. 1. Else, 1. If _roundingPriority_ is *"morePrecision"*, set _intlObj_.[[RoundingType]] to ~morePrecision~. 1. Else if _roundingPriority_ is *"lessPrecision"*, set _intlObj_.[[RoundingType]] to ~lessPrecision~. 1. Else if _hasSd_ is *true*, set _intlObj_.[[RoundingType]] to ~significantDigits~. 1. Else, set _intlObj_.[[RoundingType]] to ~fractionDigits~.

spec/numberformat.html

gibson042 · 2023-04-04T20:01:23Z

spec/numberformat.html

+        1. If _nf_.[[RoundingType]] is ~morePrecision~, then
+          1. Perform ! CreateDataPropertyOrThrow(_options_, *"roundingPriority"*, *"morePrecision"*).
+        1. Else if _nf_.[[RoundingType]] is ~lessPrecision~, then
+          1. Perform ! CreateDataPropertyOrThrow(_options_, *"roundingPriority"*, *"lessPrecision"*).
+        1. Else,
+          1. Perform ! CreateDataPropertyOrThrow(_options_, *"roundingPriority"*, *"auto"*).


Would it make more sense to just introduce a [[RoundingPriority]] slot and avoid the potential of this recovery logic falling out of sync in the future (and also the minor weirdness of "roundingPriority" being required to appear as the last property of the object returned from resolvedOptions())?

Would rather not make this big of a change at this point. Also, this logic reflects that RoundingPriority is now a computed property: if compact notation is used, it gets computed as "morePrecision", which is needed for round-tripping.

Would it make more sense to just introduce a [[RoundingPriority]] slot and avoid the potential of this recovery logic falling out of sync in the future (and also the minor weirdness of "roundingPriority" being required to appear as the last property of the object returned from resolvedOptions())?

Would rather not make this big of a change at this point. Also, this logic reflects that RoundingPriority is now a computed property: if compact notation is used, it gets computed as "morePrecision", which is needed for round-tripping.

@ryzokuken Thoughts on the above? I can live with it, albeit unhappily.

spec/numberformat.html

gibson042 · 2023-04-04T20:26:05Z

spec/numberformat.html

+          </tr>
+        </tbody>
+      </table>
+      <emu-note>The examples are illustrative of the unique behaviour of each option. ➕ means "resolves toward positive infinity"; ➖ means "resolves toward negative infinity".</emu-note>


I appreciate the information, but I wonder if and collide too much with actual numeric signs. Did you consider arrows instead?

↥ and ↧

⤒ and ⤓

⬆ and ⬇️

⭱ and ⭳

⮉ and ⮋

⇧ and ⇩

etc.

Arrow emoji SGTM

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>

spec/numberformat.html

gibson042 · 2023-04-05T18:31:32Z

spec/numberformat.html

+        1. If _nf_.[[RoundingType]] is ~morePrecision~, then
+          1. Perform ! CreateDataPropertyOrThrow(_options_, *"roundingPriority"*, *"morePrecision"*).
+        1. Else if _nf_.[[RoundingType]] is ~lessPrecision~, then
+          1. Perform ! CreateDataPropertyOrThrow(_options_, *"roundingPriority"*, *"lessPrecision"*).
+        1. Else,
+          1. Perform ! CreateDataPropertyOrThrow(_options_, *"roundingPriority"*, *"auto"*).


Would it make more sense to just introduce a [[RoundingPriority]] slot and avoid the potential of this recovery logic falling out of sync in the future (and also the minor weirdness of "roundingPriority" being required to appear as the last property of the object returned from resolvedOptions())?

Would rather not make this big of a change at this point. Also, this logic reflects that RoundingPriority is now a computed property: if compact notation is used, it gets computed as "morePrecision", which is needed for round-tripping.

@ryzokuken Thoughts on the above? I can live with it, albeit unhappily.

gibson042

A few final nits, with the one applying to PartitionNumberRangePattern being a blocking concern (but easy to resolve).

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>

sffc · 2023-04-05T20:33:52Z

Thanks for the suggestions; they all looked good to me

ryzokuken · 2023-04-06T10:25:11Z

@ryzokuken Thoughts on the above? I can live with it, albeit unhappily.

@gibson042 this could be addressed by a follow-up editorial change, right? I want to kick off the opt-out ASAP and then we could clean up the working draft.

gibson042 · 2023-04-06T17:01:57Z

this could be addressed by a follow-up editorial change, right? I want to kick off the opt-out ASAP and then we could clean up the working draft.

It would only be editorial if we commit to the awkward property ordering: …, roundingMode, roundingIncrement, trailingZeroDisplay, roundingPriority.

The property ordering is already awkward, which is basically why I can live this, but my preference is still to avoid making things worse.

sffc added 3 commits January 30, 2023 09:11

Integrate Intl NumberFormat v3 into ECMA-402

267f411

Editorial: non-zero

88ad5c1

Editorial: assorted lints from ecmarkup

ed16b0d

Editorial: Add explicit variable declaration annotation

d39854d

sffc marked this pull request as ready for review January 30, 2023 19:21

sffc requested review from ryzokuken and gibson042 January 31, 2023 00:44

jridgewell mentioned this pull request Jan 31, 2023

Jan 2023 babel/proposals#86

Open

gibson042 requested changes Mar 2, 2023

View reviewed changes

spec/pluralrules.html Outdated Show resolved Hide resolved

spec/pluralrules.html Outdated Show resolved Hide resolved

spec/pluralrules.html Show resolved Hide resolved

spec/pluralrules.html Show resolved Hide resolved

ryzokuken approved these changes Apr 4, 2023

View reviewed changes

gibson042 reviewed Apr 4, 2023

View reviewed changes

sffc and others added 6 commits April 4, 2023 18:31

Apply suggestions from code review

322ecc3

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>

Restore SetNumberFormatDigitOptions structured header

bb563c7

Rearrange SetNumberFormatDigitOptions

397af1a

Arrow emoji in table

61c9d18

Minor fixes to approximately AOs

c5d325c

Fix call to ResolvePlural in relativetimeformat.html

55e31c0

sffc requested a review from gibson042 April 5, 2023 02:12

gibson042 reviewed Apr 5, 2023

View reviewed changes

gibson042 requested changes Apr 5, 2023

View reviewed changes

Apply suggestions from code review

8a72e4e

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>

sffc requested a review from gibson042 April 5, 2023 20:33

gibson042 approved these changes Apr 6, 2023

View reviewed changes

gibson042 merged commit 4257160 into master Apr 6, 2023

gibson042 deleted the proposal-nfv3 branch April 6, 2023 17:39

ben-allen pushed a commit to ben-allen/ecma402 that referenced this pull request Apr 10, 2023

Normative: Integrate Intl NumberFormat v3 into ECMA-402 (tc39#753)

c0b5b63

ben-allen pushed a commit to ben-allen/ecma402 that referenced this pull request Apr 11, 2023

Normative: Integrate Intl NumberFormat v3 into ECMA-402 (tc39#753)

5a16f08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normative: Integrate Intl NumberFormat v3 into ECMA-402 #753

Normative: Integrate Intl NumberFormat v3 into ECMA-402 #753

sffc commented Jan 30, 2023

sffc commented Jan 30, 2023

gibson042 commented Jan 30, 2023

sffc commented Jan 30, 2023

gibson042 left a comment

ryzokuken left a comment

ryzokuken Apr 4, 2023

sffc Apr 5, 2023

ryzokuken Apr 4, 2023

gibson042 Apr 4, 2023

sffc Apr 5, 2023

ryzokuken Apr 4, 2023

sffc Apr 5, 2023

ryzokuken Apr 4, 2023

sffc Apr 5, 2023

ryzokuken Apr 4, 2023

sffc Apr 5, 2023

gibson042 left a comment

gibson042 Apr 4, 2023

sffc Apr 5, 2023

gibson042 Apr 4, 2023

sffc Apr 5, 2023 •

edited

Loading

gibson042 Apr 5, 2023 •

edited by sffc

Loading

gibson042 Apr 4, 2023

sffc Apr 5, 2023

gibson042 Apr 5, 2023 •

edited by sffc

Loading

gibson042 left a comment

sffc commented Apr 5, 2023

ryzokuken commented Apr 6, 2023

gibson042 commented Apr 6, 2023 •

edited

Loading

Normative: Integrate Intl NumberFormat v3 into ECMA-402 #753

Normative: Integrate Intl NumberFormat v3 into ECMA-402 #753

Conversation

sffc commented Jan 30, 2023

sffc commented Jan 30, 2023

gibson042 commented Jan 30, 2023

sffc commented Jan 30, 2023

gibson042 left a comment

Choose a reason for hiding this comment

ryzokuken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sffc Apr 5, 2023 • edited Loading

Choose a reason for hiding this comment

gibson042 Apr 5, 2023 • edited by sffc Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 Apr 5, 2023 • edited by sffc Loading

Choose a reason for hiding this comment

gibson042 left a comment

Choose a reason for hiding this comment

sffc commented Apr 5, 2023

ryzokuken commented Apr 6, 2023

gibson042 commented Apr 6, 2023 • edited Loading

sffc Apr 5, 2023 •

edited

Loading

gibson042 Apr 5, 2023 •

edited by sffc

Loading

gibson042 Apr 5, 2023 •

edited by sffc

Loading

gibson042 commented Apr 6, 2023 •

edited

Loading