Skip to content

Editorial: A few spec text fixes found during implementation#126

Open
trflynn89 wants to merge 4 commits intotc39:mainfrom
trflynn89:fixes
Open

Editorial: A few spec text fixes found during implementation#126
trflynn89 wants to merge 4 commits intotc39:mainfrom
trflynn89:fixes

Conversation

@trflynn89
Copy link
Copy Markdown

Noticed a few things while implementing non-ISO-8601 support for LibJS.

We just constrained the month before this step.
This step was the only one which did not end with "to ignoredKeys".
The only caller (CalendarDateAdd) performs this check on the result.
Copy link
Copy Markdown
Collaborator

@ptomato ptomato left a comment

Choose a reason for hiding this comment

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

Thanks for catching these! Other than the NonISODateAdd change, I'll tack them on to the stage 4 PR.

1. Let _result_ be ? CalendarIntegersToISO(_calendar_, _balancedDate_.[[Year]], _balancedDate_.[[Month]], _balancedDate_.[[Day]]).
1. If ISODateWithinLimits(_result_) is *false*, throw a *RangeError* exception.
1. Return _result_.
1. Return ? CalendarIntegersToISO(_calendar_, _balancedDate_.[[Year]], _balancedDate_.[[Month]], _balancedDate_.[[Day]]).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd prefer to keep this as is, in analogy to CalendarDateAdd.

1. Else,
1. Let _month_ be _fields_.[[Month]].
1. Let _daysInMonth_ be CalendarDaysInMonth(_calendar_, _fields_.[[Year]], _fields_.[[Month]]).
1. Let _daysInMonth_ be CalendarDaysInMonth(_calendar_, _fields_.[[Year]], _month_).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

(This one is already fixed in the stage 4 PR. I must have forgotten to port it back here.)

ptomato added a commit to ptomato/ecma402 that referenced this pull request Mar 10, 2026
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