Various editorial changes #2207
Conversation
CalendarDateFromFields already sets the options argument to undefined if it is not present, so omit it for clarity.
…t V3 We intend to standardize on the same rounding modes as Intl.NumberFormat (see #1038), so it makes sense to use the same abstract operations, namely GetUnsignedRoundingMode and ApplyUnsignedRoundingMode. Note that this does not yet add user-visible support for the full set of rounding modes that Intl.NumberFormat does; that (#1038) is a normative change that we'll apply once Intl.NumberFormat goes to Stage 4. However, this way of expressing rounding makes it easier to make that change in the future, and makes it easier to address #2191 in the short term. RoundTowardsZero stays the same, since that is used for several other things as well as rounding according to a rounding mode.
Codecov Report
@@ Coverage Diff @@
## main #2207 +/- ##
=======================================
Coverage 89.95% 89.95%
=======================================
Files 19 19
Lines 10901 10901
Branches 1683 1683
=======================================
Hits 9806 9806
Misses 1081 1081
Partials 14 14
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
spec/timezone.html
Outdated
| @@ -332,7 +324,7 @@ <h1>Temporal.TimeZone.prototype.getPreviousTransition ( _startingPoint_ )</h1> | |||
| <emu-clause id="sec-temporal.timezone.prototype.tostring"> | |||
| <h1>Temporal.TimeZone.prototype.toString ( )</h1> | |||
| <p> | |||
| The following steps are taken: | |||
| The `Temporal.TimeZone.prototype.toString` function performs the following steps when called: | |||
There was a problem hiding this comment.
Is this not a method rather than a function, if we wanna be pedantic?
There was a problem hiding this comment.
To be pedantic, nothing’s really a method, they’re all just function-valued properties :-) but typically yes we’d describe receiver-sensitive functions as a method.
However, the spec usually just says “the following steps are performed” - why do we need to repeat the name of the function/method here?
There was a problem hiding this comment.
That's what is asked for in tc39/ecma262#2304.
There was a problem hiding this comment.
That’s just one of the editors’ two acceptable options. It seems very redundant to me, and either way it’s probably best not to apply a new style before it lands in the actual spec.
There was a problem hiding this comment.
I only see one agreed-on option there? It has two variations, one without any additional informative description, and one with.
There was a problem hiding this comment.
The "…following steps…" changes make for a lot of noise. I think I'd like to see them in a separate PR (ideally switching to structured headers rather than tweaking text), leaving this PR consisting of just CalendarDateFromFields(calendar, fields, *undefined*), Intl.NumberFormat v3 alignment, and emu-clause id corrections.
But that said, everything here does look good AFAICT.
| <dl class="header"> | ||
| <dt>description</dt> | ||
| <dd> | ||
| The return value is the rounding mode that should be applied to the absolute value of a number to produce the same result as if _roundingMode_ were applied to the signed value of the number (negative if _isNegative_ is *true*, or positive otherwise). |
There was a problem hiding this comment.
I find this description confusing, but if it matches Intl.NumberFormat v3 then I guess that would be the place to address it.
| <thead> | ||
| <tr> | ||
| <th>Identifier</th> | ||
| <th>Sign</th> | ||
| <th>Unsigned Rounding Mode</th> | ||
| </tr> | ||
| </thead> |
There was a problem hiding this comment.
Since Sign is always "positive" or "negative", wouldn't an alternate arrangement be more clear?
Identifier Non-negative Rounding Mode Negative Rounding Mode "ceil" ~infinity~ ~zero~ "floor" ~zero~ ~infinity~ … … …
There was a problem hiding this comment.
This should probably be addressed in NFv3 as well? @sffc, what do you think?
spec/instant.html
Outdated
| @@ -26,7 +26,7 @@ <h1>The Temporal.Instant Constructor</h1> | |||
| <emu-clause id="sec-temporal.instant"> | |||
| <h1>Temporal.Instant ( _epochNanoseconds_ )</h1> | |||
| <p> | |||
| When the `Temporal.Instant` function is called, the following steps are taken: | |||
| The `Temporal.Instant` function performs the following steps when called: | |||
There was a problem hiding this comment.
Are these changes really helpful? It seems like switching to structured headers (in which such raw spec text just goes away) would be better.
There was a problem hiding this comment.
There are no structured headers for functions yet, only for abstract operations.
| <h1>RoundHalfAwayFromZero ( _x_ )</h1> | ||
| <emu-clause id="sec-temporal-getunsignedroundingmode" type="abstract operation"> | ||
| <h1> | ||
| GetUnsignedRoundingMode ( |
There was a problem hiding this comment.
Question: With this AO landing in Temporal 262, I assume that Intl.NumberFormat should reference this AO instead of including it in its own proposal?
If NFv3 lands first (which seems likely -- it's late Stage 3), should it add these AOs into 262, or should it keep them in 402, and Temporal moves them?
There was a problem hiding this comment.
Either approach seems workable, but NFv3 should indeed contain an inline implementation of the AO.
There was a problem hiding this comment.
It seems less work overall if NFv2 adds these operations into 262 already.
There was a problem hiding this comment.
I think I prefer NFv3 keeping these AOs in 402, as planned, and Temporal moves them, because NFv3 does not currently touch 262 at all, but Temporal is touching both 402 and 262. It seems easy enough to add it as a diff here.
It seems like the ID should use dots here, not dashes, in order to be consistent with other clauses that describe the values of properties.
|
Removed the standard prose changes |
Finish standardizing the prose describing functions and methods (rest of Make introduction paragraph for functions consistent in spec text #1465)Closes: #1465