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.

Sign up for GitHub

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

Jump to bottom

Editorial: Refactor SetNumberFormatDigitOptions #575

Merged
merged 13 commits into from
Oct 12, 2021
Merged

Editorial: Refactor SetNumberFormatDigitOptions #575

Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0e3ce45
Editorial: Refactor SetNumberFormatDigitOptions
sffc May 15, 2021
902f943
Additional refactor of [[RoundingType]]
sffc May 16, 2021
16601cf
Review feedback
sffc Jun 18, 2021
d9ef6c5
More review feedback
sffc Aug 13, 2021
68b8600
Apply suggestions from code review
sffc Aug 16, 2021
e7d53bc
Apply suggestions from code review
sffc Aug 16, 2021
31b6a64
More Set -> Let
sffc Aug 16, 2021
06e43cf
Let to -> Let be (thanks @ptomato!)
sffc Aug 16, 2021
dd7d0cf
Fix logic
sffc Aug 16, 2021
a3728b9
Adopt structured header
sffc Aug 16, 2021
ec70f0e
Apply suggestions from code review
sffc Aug 30, 2021
5272398
Additional feedback from @gibson042
sffc Aug 30, 2021
cd6a927
Update spec/numberformat.html
sffc Oct 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 47 additions & 23 deletions spec/numberformat.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,19 @@ <h1>NumberFormat Objects</h1>
<h1>Abstract Operations for NumberFormat Objects</h1>

<emu-clause id="sec-setnfdigitoptions" aoid="SetNumberFormatDigitOptions">
sffc marked this conversation as resolved.
Show resolved Hide resolved
<h1>SetNumberFormatDigitOptions ( _intlObj_, _options_, _mnfdDefault_, _mxfdDefault_, _notation_ )</h1>

<p>
The abstract operation SetNumberFormatDigitOptions applies digit
options used for number formatting onto the intl object.
</p>
<h1>
SetNumberFormatDigitOptions (
_intlObj_: an Object,
_options_: an Object,
_mnfdDefault_: a Number,
_mxfdDefault_: a Number,
_notation_: a String,
)
</h1>
<dl class="header">
<dt>description</dt>
<dd>It resolves fraction digits and significant digits used for number formatting onto the intl object.</dd>
sffc marked this conversation as resolved.
Show resolved Hide resolved
</dl>
<emu-alg>
1. Assert: Type(_intlObj_) is Object.
1. Assert: Type(_options_) is Object.
sffc marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -23,26 +30,43 @@ <h1>SetNumberFormatDigitOptions ( _intlObj_, _options_, _mnfdDefault_, _mxfdDefa
1. Let _mxsd_ be ? Get(_options_, *"maximumSignificantDigits"*).
1. Set _intlObj_.[[MinimumIntegerDigits]] to _mnid_.
1. If _mnsd_ is not *undefined* or _mxsd_ is not *undefined*, then
1. Set _intlObj_.[[RoundingType]] to ~significantDigits~.
1. Let _mnsd_ be ? DefaultNumberOption(_mnsd_, 1, 21, 1).
1. Let _mxsd_ be ? DefaultNumberOption(_mxsd_, _mnsd_, 21, 21).
1. Let _hasSd_ be *true*.
1. Else,
1. Let _hasSd_ be *false*.
1. If _mnfd_ is not *undefined* or _mxfd_ is not *undefined*, then
1. Let _hasFd_ be *true*.
1. Else,
1. Let _hasFd_ be *false*.
1. Let _needSd_ be _hasSd_.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the motivation for realiasing hasSd rather than continuing to use it directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the parallelism between [has/need]Fd with [has/need]Sd.

Also, in NumberFormat v3, needSd no longer equals hasSd. I could merge the variables for now, but doing so will increase the diff relative to NFv3.

1. If _hasSd_ is *false* and (_hasFd_ is *true* or _notation_ is not *"compact"*), then
1. Let _needFd_ be *true*.
1. Else,
1. Let _needFd_ be *false*.
sffc marked this conversation as resolved.
Show resolved Hide resolved
1. If _needSd_ is *true*, then
1. Assert: _hasSd_ is *true*.
1. Set _mnsd_ to ? DefaultNumberOption(_mnsd_, 1, 21, 1).
1. Set _mxsd_ to ? DefaultNumberOption(_mxsd_, _mnsd_, 21, 21).
1. Set _intlObj_.[[MinimumSignificantDigits]] to _mnsd_.
1. Set _intlObj_.[[MaximumSignificantDigits]] to _mxsd_.
1. Else if _mnfd_ is not *undefined* or _mxfd_ is not *undefined*, then
1. Set _intlObj_.[[RoundingType]] to ~fractionDigits~.
1. Let _mnfd_ be ? DefaultNumberOption(_mnfd_, 0, 20, *undefined*).
1. Let _mxfd_ be ? DefaultNumberOption(_mxfd_, 0, 20, *undefined*).
1. If _mnfd_ is *undefined*, set _mnfd_ to min(_mnfdDefault_, _mxfd_).
1. Else if _mxfd_ is *undefined*, set _mxfd_ to max(_mxfdDefault_, _mnfd_).
1. Else if _mnfd_ is greater than _mxfd_, throw a *RangeError* exception.
1. Set _intlObj_.[[MinimumFractionDigits]] to _mnfd_.
1. Set _intlObj_.[[MaximumFractionDigits]] to _mxfd_.
1. Else if _notation_ is *"compact"*, then
1. Set _intlObj_.[[RoundingType]] to ~compactRounding~.
1. If _needFd_ is *true*, then
1. If _hasFd_ is *true*, then
1. Set _mnfd_ to ? DefaultNumberOption(_mnfd_, 0, 20, *undefined*).
1. Set _mxfd_ to ? DefaultNumberOption(_mxfd_, 0, 20, *undefined*).
1. If _mnfd_ is *undefined*, set _mnfd_ to min(_mnfdDefault_, _mxfd_).
1. Else if _mxfd_ is *undefined*, set _mxfd_ to max(_mxfdDefault_, _mnfd_).
1. Else if _mnfd_ is greater than _mxfd_, throw a *RangeError* exception.
1. Set _intlObj_.[[MinimumFractionDigits]] to _mnfd_.
1. Set _intlObj_.[[MaximumFractionDigits]] to _mxfd_.
1. Else,
1. Set _intlObj_.[[MinimumFractionDigits]] to _mnfdDefault_.
1. Set _intlObj_.[[MaximumFractionDigits]] to _mxfdDefault_.
1. If _needSd_ or _needFd_, then
1. If _hasSd_, then
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. If _needSd_ or _needFd_, then
1. If _hasSd_, then
1. If _needSd_ is *true* or _needFd_ is *true*, then
1. If _hasSd_ is *true*, then

But for that matter, this part seems hard to follow. Broader suggestion:

        1. If _needSd_ is *false* and _needFd_ is *false*, then
            1. Set _intlObj_.[[RoundingType]] to ~compactRounding~.
        1. Else if _hasSd_ is *true*, then
            1. Set _intlObj_.[[RoundingType]] to ~significantDigits~.
        1. Else,
            1. Set _intlObj_.[[RoundingType]] to ~fractionDigits~.

1. Set _intlObj_.[[RoundingType]] to ~significantDigits~.
1. Else,
1. Set _intlObj_.[[RoundingType]] to ~fractionDigits~.
1. Else,
1. Set _intlObj_.[[RoundingType]] to ~fractionDigits~.
1. Set _intlObj_.[[MinimumFractionDigits]] to _mnfdDefault_.
1. Set _intlObj_.[[MaximumFractionDigits]] to _mxfdDefault_.
1. Set _intlObj_.[[RoundingType]] to ~compactRounding~.
</emu-alg>
</emu-clause>

Expand Down