-
Notifications
You must be signed in to change notification settings - Fork 32
Rebase: ISO Date-Time Record refactoring #316
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.
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
Merged
Conversation
This file contains hidden or 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
We test for the Time Record being ~start-of-day~ here, but that cannot happen; InterpretTemporalDateTimeFields cannot return that value. Closes: #3000 UPSTREAM_COMMIT=ced117393288d1cd0a621b0077c9716b710f0a35
Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=7d3fe7194511d68414757851f4e5673696fe669d
In CompareISODate, use two ISO Date Records instead of passing each component individually. In ISODateSurpasses, the first date does not have to exist, so rewrite the operation without using CompareISODate and only take an ISO Date Record for the second parameter. See: #2949 UPSTREAM_COMMIT=184cc3366d7ce068e3346fb5b9a8eeab9316e32f
Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=a27ab8999190c18191d69fb297f52e6d890cc266
Instead of passing each component individually, pass a Time Record. The name is no longer really appropriate, since we are not passing a Temporal object, so rename it to TimeRecordToString. See: #2949 Co-authored-by: Ms2ger <[email protected]> UPSTREAM_COMMIT=218ff5a25d03bc80c433aab9d1a888e4069a4dab
Instead of passing each component individually, pass Time Records. The name is no longer really appropriate, since we are not passing a Temporal object, so rename it to CompareTimeRecord. See: #2949 UPSTREAM_COMMIT=2d107444d7e85e3bc010d0dc44032ed2900275d6
Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=535cbacd66726d4781e95b7eb412f05b1ed743ab
Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=2cd4372e18ea8f2d43e82c00f5a0acb26ac80417
Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=45f82f9e8ec147b87efcef7d6654294f787e8e34
…rename Instead of passing each component individually, pass an ISO Date-Time Record. The name is no longer really appropriate, since we are not passing a Temporal object, so rename it to ISODateTimeToString. See: #2949 UPSTREAM_COMMIT=991d88262315c1ec1eaa3fe5ef7e198fd552f836
…ecord Instead of individual fields, give ISO Date-Time Record just two fields, [[ISODate]] and [[Time]]. Since we are passing ISO Date and Time Records in more places, this is better for readability. See: #2949 UPSTREAM_COMMIT=a11d31cde12f6c4efb7b6b49c54898688468bc0b
Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=aa3a36ec3671927ce17120e7e34bc3e1a708e1cc
Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=57350a3092b28af13e76311dc6982cbba2490adb
…nding Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=1452c0999a615a4b21616499a62d5f42766b3e52
Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=02f8c43911d89f30550d3c8d7ae04710a8968432
Instead of passing each component individually. See: #2949 UPSTREAM_COMMIT=5396508f31cc9909836e8fb4b57224212fad7ee3
Instead of passing each component individually. The [[Day]] field is ignored. See: #2949 UPSTREAM_COMMIT=9e801047ea50e8ecb3f45bac3b1ae13a0a87cbd2
Instead of passing each component individually. GetUTCEpochNanoseconds is part of ECMA-262 already, so this requires some <ins> and <del> tags. See: #2949 UPSTREAM_COMMIT=e948cc8d4eed7b1602e8f54239bd15113d0e594d
Instead of passing each component individually. GetNamedTimeZoneEpochNanoseconds is part of ECMA-262 already, so this requires some <ins> and <del> tags. See: #2949 UPSTREAM_COMMIT=8e57e867567a76aa6dc22cd21bd901170066a878
These are repeated several times now throughout the spec, so abstract them out for readability. See: #2949 UPSTREAM_COMMIT=1e06584293f6ffbd7332372bcb6fedee1f5054c8
For PlainDate, PlainYearMonth, and PlainMonthDay, use an ISO Date Record in the [[ISODate]] internal slot. For PlainDateTime, use an ISO Date-Time Record in the [[ISODateTime]] internal slot. For PlainTime, use a Time Record in the [[Time]] internal slot. This makes a lot more concise spec text, and probably corresponds better with how it will be implemented. See: #2949 Co-authored-by: Justin Grant <[email protected]> Co-authored-by: Ms2ger <[email protected]> UPSTREAM_COMMIT=86ff1109883908c0b81ed4468bfc07fb056e90c3
This is the only place it's called from, and it consolidates operations that take individual time components. See: #2949 UPSTREAM_COMMIT=abb2f2cbdf830f8096f4070a767463a0c3f03466
This avoids creation of an unobservable Temporal.PlainDateTime object, and is also simpler to read. See: #2949 UPSTREAM_COMMIT=1a23f2c9f2c044ea0053b064e30bb44a5e200cf0
Now that it's trivial to get an ISO Date Record for a Temporal object, we don't really need this operation. All the switching on Temporal object type that it does, is already known at each call site. UPSTREAM_COMMIT=59a95b429cad320c092f5a28cc21c2459de70937
This avoids creation of an unobservable Temporal.PlainTime object, and is also simpler to read. Since it no longer returns a Temporal object, give it a more appropriate name. UPSTREAM_COMMIT=dd9679fea7db0130738cf8636b47d2eb67e4afe9
justingrant
approved these changes
Mar 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
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.
Here is a batch of commits from October 2024 all dealing with refactoring the way ISO Date-Time Records are used in the spec. It made the spec text more concise, so should do the same with this JS code. (Net -1k lines)