Skip to content

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
merged 25 commits into from
Mar 22, 2025
Merged

Rebase: ISO Date-Time Record refactoring #316

merged 25 commits into from
Mar 22, 2025

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Mar 20, 2025

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)

ptomato added 25 commits March 19, 2025 19:15
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
@ptomato ptomato requested review from justingrant and 12wrigja March 20, 2025 02:17
Copy link
Contributor

@justingrant justingrant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@ptomato ptomato merged commit 41f2072 into main Mar 22, 2025
19 checks passed
@ptomato ptomato deleted the rebase branch March 22, 2025 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants