Editorial: Refactor add/subtract to common AOs #2166
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2166 +/- ##
==========================================
- Coverage 90.03% 90.00% -0.03%
==========================================
Files 19 19
Lines 10905 10905
Branches 1693 1686 -7
==========================================
- Hits 9818 9815 -3
- Misses 1073 1076 +3
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/duration.html
Outdated
| 1. Let _relativeTo_ be ? ToRelativeTemporalObject(_options_). | ||
| 1. Let _result_ be ? AddDuration(_duration_.[[Years]], _duration_.[[Months]], _duration_.[[Weeks]], _duration_.[[Days]], _duration_.[[Hours]], _duration_.[[Minutes]], _duration_.[[Seconds]], _duration_.[[Milliseconds]], _duration_.[[Microseconds]], _duration_.[[Nanoseconds]], _other_.[[Years]], _other_.[[Months]], _other_.[[Weeks]], _other_.[[Days]], _other_.[[Hours]], _other_.[[Minutes]], _other_.[[Seconds]], _other_.[[Milliseconds]], _other_.[[Microseconds]], _other_.[[Nanoseconds]], _relativeTo_). | ||
| 1. Return ! CreateTemporalDuration(_result_.[[Years]], _result_.[[Months]], _result_.[[Weeks]], _result_.[[Days]], _result_.[[Hours]], _result_.[[Minutes]], _result_.[[Seconds]], _result_.[[Milliseconds]], _result_.[[Microseconds]], _result_.[[Nanoseconds]]). | ||
| 1. Return ! AddTemporalDuration(1, _duration_, _other_, _options_). |
There was a problem hiding this comment.
if all of the removed lines can throw, how come this one can't?
There was a problem hiding this comment.
opps. sorry, forget to change that.
31d62a6 to
ce05e4f
Compare
|
PTAL |
|
LGTM! |
There was a problem hiding this comment.
Thanks! This looks like an improvement.
I don't think the <h1>s will render correctly unless we have type="abstract operation" in the <emu-clause> and also the <dl class="header"> element. (See https://tc39.es/ecmarkup/#emu-clause-structured-headers)
Since a description is needed in the <dl class="header"> element, I think it's probably good to make clear what the distinction is between e.g. AddTemporalInstant and AddInstant. So, for the description of AddTemporalInstant you could say something like "It implements the validation of the arguments and return value for Temporal.Instant.prototype.add and Temporal.Instant.prototype.subtract, around a call to AddInstant which performs the actual arithmetic.", etc. for the other types.
As for the -1/+1 vs enumeration, I commented on #2167 - let's see if anyone has more to say on that.
|
ok, I correct the structure header, change to use enum and improve the readability of the AO name |
|
LGTM |
Co-authored-by: Philip Chimento <philip.chimento@gmail.com>
Co-authored-by: Philip Chimento <philip.chimento@gmail.com>
Co-authored-by: Philip Chimento <philip.chimento@gmail.com>
Co-authored-by: Philip Chimento <philip.chimento@gmail.com>
|
sorry, my bad. Thaks @ptomato for the correction |
…PlainTime This fixes a 'normative typo' introduced in tc39#2166. Instead of passing temporalTime twice, we need to also pass temporalDurationLike.
* Editorial: Rename AddTemporalPlainYearMonth to AddDurationToOrSubtractDurationFromPlainYearMonth This AO has both addition and subtraction behavior, based on the first argument. Update the name accordingly and bring it in line with these similar AOs: - AddDurationToOrSubtractDurationFromDuration - AddDurationToOrSubtractDurationFromInstant - AddDurationToOrSubtractDurationFromPlainDateTime - AddDurationToOrSubtractDurationFromPlainTime - AddDurationToOrSubtractDurationFromZonedDateTime * Editorial: Fix rendering of 'subtract' in structured header * Editorial: Pass correct values to AddDurationToOrSubtractDurationFromPlainTime This fixes a 'normative typo' introduced in #2166. Instead of passing temporalTime twice, we need to also pass temporalDurationLike.
Most of the operations between add and subtract are the same except negative value. Refactor to shared AOs between them with a sign which either 1 or -1.