Normative: Use singular unit names throughout #1509
Conversation
|
Draft while still missing test262 tests. (edit: also to avoid merging it accidentally before TC39 consensus) |
Codecov Report
@@ Coverage Diff @@
## main #1509 +/- ##
==========================================
- Coverage 95.59% 95.57% -0.02%
==========================================
Files 19 19
Lines 11139 11139
Branches 1736 1736
==========================================
- Hits 10648 10646 -2
- Misses 485 487 +2
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
ptomato
added
the
needs plenary input
ptomato
force-pushed
the
1491-singular-unit-names
branch
from
2041512 to
f4a0db9
Compare
Both singular and plural names remain accepted everywhere, and 'week' is now accepted where previously only 'weeks' was. However, the singular form is now preferred for Temporal.Duration as well, and the spec text should reflect this. Ensure that options objects passed to user code have the correct singular unit strings on them. Closes: #1491
ptomato
force-pushed
the
1491-singular-unit-names
branch
from
f4a0db9 to
dcadb53
Compare
| 1. Perform ? ValidateTemporalUnitRange(_largestUnit_, _smallestUnit_). | ||
| 1. Let _roundingMode_ be ? ToTemporalRoundingMode(_options_, *"trunc"*). | ||
| 1. Set _roundingMode_ to ! NegateTemporalRoundingMode(_roundingMode_). | ||
| 1. Let _roundingIncrement_ be ? ToTemporalRoundingIncrement(_options_, *undefined*, *false*). | ||
| 1. Let _result_ be ? CalendarDateUntil(_temporalDate_.[[Calendar]], _other_, _temporalDate_, _options_). | ||
| 1. If _smallestUnit_ is *"days"* and _roundingIncrement_ = 1, then | ||
| 1. Let _untilOptions_ be ? MergeLargestUnitOption(_options_, _largestUnit_). |
There was a problem hiding this comment.
@Ms2ger @justingrant Relative to the version you previously reviewed, I added this MergeLargestUnitOption invocation to PlainDate.until and PlainDate.since, which essentially passes { ...options, largestUnit } to Calendar.dateUntil, instead of options directly. I decided to do this after writing some test262 tests, so that dateUntil always receives an options bag with a normalized largestUnit, instead of having that depend on which Temporal method you call.
This is the implementation of the normative changes in #1509 in the polyfill, accompanied by test262 tests to ensure that passing plural and singular values for largestUnit, smallestUnit, and unit behaves the same; and also that Calendar.dateUntil() is only ever called with options bags that contain singular values for largestUnit. See: #1491
There was a problem hiding this comment.
ToLargestTemporalUnit has 3 args as stated in https://tc39.es/proposal-temporal/#sec-temporal-tolargesttemporalunit but this PR put 4 args here.
1. Let _largestUnit_ be ? ToLargestTemporalUnit(_options_, « .... », *"auto"*, *"day"*).
not clear how to interpret the "auto" and "day" as the last two args here.
|
@FrankYFTang This PR also changes ToLargestTemporalUnit accordingly to have 4 args (1 of which is optional). |
sorry. I was blinded by the hiding of large changes done by github. |
|
Achieved consensus in today's TC39 meeting. |
This is the implementation of the normative changes in #1509 in the polyfill, accompanied by test262 tests to ensure that passing plural and singular values for largestUnit, smallestUnit, and unit behaves the same; and also that Calendar.dateUntil() is only ever called with options bags that contain singular values for largestUnit. See: #1491
|
I don't think we have to wait on #1519 to merge this, so I'll go ahead. |
This is the implementation of the normative changes in #1509 in the polyfill, accompanied by test262 tests to ensure that passing plural and singular values for largestUnit, smallestUnit, and unit behaves the same; and also that Calendar.dateUntil() is only ever called with options bags that contain singular values for largestUnit. See: #1491
This is the implementation of the normative changes in #1509 in the polyfill, accompanied by test262 tests to ensure that passing plural and singular values for largestUnit, smallestUnit, and unit behaves the same; and also that Calendar.dateUntil() is only ever called with options bags that contain singular values for largestUnit. See: #1491
This is the implementation of the normative changes in #1509 in the polyfill, accompanied by test262 tests to ensure that passing plural and singular values for largestUnit, smallestUnit, and unit behaves the same; and also that Calendar.dateUntil() is only ever called with options bags that contain singular values for largestUnit. See: #1491
|
Whoo-hoo! Sorry I haven't gotten to the Node 14 failures yet... was away camping with the family last weekend. Will take a look at it this week. |
Both singular and plural names remain accepted everywhere, and 'week' is
now accepted where previously only 'weeks' was. However, the singular form
is now preferred for Temporal.Duration as well, and the spec text should
reflect this.
Ensure that options objects passed to user code have the correct singular
unit strings on them.
Closes: #1491