Normative: Fix order of rounding* option reads and resolvedOptions() #811
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The fix for the lint error is in #812 |
ryzokuken
approved these changes
Aug 3, 2023
|
TG2 Discussion: https://github.com/tc39/ecma402/blob/master/meetings/notes-2023-08-03.md#normative-fix-order-of-rounding-option-reads-and-resolvedoptions-811 Conclusion: RGN and FYT will work together on changes to the PR, which we will then bring to plenary. |
|
Ping @FrankYFTang @gibson042 |
1 similar comment
|
Ping @FrankYFTang @gibson042 |
FrankYFTang
added a commit
to FrankYFTang/ecma402
that referenced
this pull request
Aug 24, 2023
Split the editorial fix from tc39#811 1. Add text to explain ComputedRoundingPriority in section "15.4 Properties of Intl.NumberFormat Instances" https://tc39.es/ecma402/#sec-properties-of-intl-numberformat-instances and "16.4 Properties of Intl.PluralRules Instances https://tc39.es/ecma402/#sec-properties-of-intl-pluralrules-instances 2. Add ComputedRoundingPriority to Intl.PluralRules https://tc39.es/ecma402/#sec-intl.pluralrules Without this, the spec is not implementable because InitializePluralRules call "SetNumberFormatDigitOptions(pluralRules, options, +0𝔽, 3𝔽, "standard")." and SetNumberFormatDigitOptions assume the intlObj has [[ComputedRoundingPriority]]
|
During the discussion of https://github.com/tc39/ecma402/blob/master/meetings/notes-2023-08-03.md#normative-fix-order-of-rounding-option-reads-and-resolvedoptions-811 RGN express the desire to split out the "editorial" part of the PR to another one. I split the item 1. and 2. into #829 now. Once we merge 829, I will rebase this PR on top of that and those editoral change should be removed from this PR. |
This PR are mostly Normative but have some editorial parts 1. Editorial: Add text to explain ComputedRoundingPriority in section "15.4 Properties of Intl.NumberFormat Instances" https://tc39.es/ecma402/#sec-properties-of-intl-numberformat-instances and "16.4 Properties of Intl.PluralRules Instances https://tc39.es/ecma402/#sec-properties-of-intl-pluralrules-instances 2. Editorial: Add ComputedRoundingPriority to Intl.PluralRules https://tc39.es/ecma402/#sec-intl.pluralrules Without this, the spec is not implementable because InitializePluralRules call "SetNumberFormatDigitOptions(pluralRules, options, +0𝔽, 3𝔽, "standard")." and SetNumberFormatDigitOptions assume the intlObj has `[[ComputedRoundingPriority]]` 3. Normative: Move the reading of "roundingPriority" from before reading "roundingIncrement" to after reading "roundingMode" so these three fields are read in alphabetic order. https://tc39.es/ecma402/#sec-setnfdigitoptions 4. Normative: Move the output order of "roundingIncrement" from after output "roundingMode" to before that so these two fields are read in alphabetic order in the resolvedOptions of both Intl.NumberFormat and Intl.PluralRules. https://tc39.es/ecma402/#sec-intl.numberformat.prototype.resolvedoptions https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions 5. Normative: Move the output of "roundingPriority" from after "trailingZeroDisplay" before that to sync w/ Intl.NumberFormat after PR768. https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions 6. Normative: Move the output of "pluralCategories" from after "trailingZeroDisplay" to before "roundingIncrement" so these options will be in alphabetic order. https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions
FrankYFTang
force-pushed
the
ReorderRounding
branch
from
6f0db10
to
9b7711a
Compare
gibson042
pushed a commit
that referenced
this pull request
Sep 7, 2023
gibson042
approved these changes
Sep 7, 2023
|
PTAL |
|
v8 implementation https://chromium-review.googlesource.com/c/v8/v8/+/4855044 |
|
gibson042
approved these changes
Sep 11, 2023
ben-allen
approved these changes
Sep 26, 2023
|
This PR reach consensus on 2023-09-26 meeting |
|
please merge |
|
@ben-allen @gibson042 Please merge. This has TC39 blessing last week. |
jamestiotio
pushed a commit
to jamestiotio/v8
that referenced
this pull request
Oct 10, 2023
Fix order of rounding* option reads and resolvedOptions() of NumberFormat and PluralRules tc39/ecma402#811 Details: Change order of rounding* option reads and resolvedOptions() 1. Move the reading of "roundingPriority" from before reading "roundingIncrement" to after reading "roundingMode" so these three fields are read in alphabetic order. https://tc39.es/ecma402/#sec-setnfdigitoptions 2. Move the output order of "roundingIncrement" from after output "roundingMode" to before that so these two fields are read in alphabetic order in the resolvedOptions of both Intl.NumberFormat and Intl.PluralRules. https://tc39.es/ecma402/#sec-intl.numberformat.prototype.resolvedoptions https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions 3. Move the output of "roundingPriority" from after "trailingZeroDisplay" before that to sync w/ Intl.NumberFormat after PR768. https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions 4. Move the output of "pluralCategories" from after "trailingZeroDisplay" to before "roundingIncrement" so these options will be in alphabetic order. https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions Bug: v8:14308 Change-Id: I37168bf0a6e03c0459c723c98feb8b34d95413fa Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4855044 Reviewed-by: Shu-yu Guo <syg@chromium.org> Commit-Queue: Frank Tang <ftang@chromium.org> Cr-Commit-Position: refs/heads/main@{#90319}
ben-allen
pushed a commit
to ben-allen/ecma402
that referenced
this pull request
Oct 12, 2023
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Mar 12, 2024
… r=dminor Implements the changes from <tc39/ecma402#811>. Differential Revision: https://phabricator.services.mozilla.com/D204013
ErichDonGubler
pushed a commit
to ErichDonGubler/firefox
that referenced
this pull request
Mar 14, 2024
… r=dminor Implements the changes from <tc39/ecma402#811>. Differential Revision: https://phabricator.services.mozilla.com/D204013
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixing issues created by #768 and additional issues about 'order' related to number format v3 and option read/ resolvedOptoins()
Move the reading of "roundingPriority" from before reading "roundingIncrement" to after reading "roundingMode" so these three fields are read in alphabetic order. https://tc39.es/ecma402/#sec-setnfdigitoptions
Move the output order of "roundingIncrement" from after output "roundingMode" to before that so these two fields are read in alphabetic order in the resolvedOptions of both Intl.NumberFormat and Intl.PluralRules. https://tc39.es/ecma402/#sec-intl.numberformat.prototype.resolvedoptions https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions
Move the output of "roundingPriority" from after "trailingZeroDisplay" before that to sync w/ Intl.NumberFormat after PR768. https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions
Move the output of "pluralCategories" from after "trailingZeroDisplay" to before "roundingIncrement" so these options will be in alphabetic order. https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions