Skip to content
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

Next batch of editorial issues #2934

Merged
merged 15 commits into from
Sep 12, 2024
Merged

Next batch of editorial issues #2934

merged 15 commits into from
Sep 12, 2024

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Sep 10, 2024

Best reviewed commit by commit. Explanations in each commit message. Closes several issues opened by @anba and others.

@ptomato
Copy link
Collaborator Author

ptomato commented Sep 10, 2024

Requires tc39/test262#4220 for the tests to pass.

@ptomato ptomato force-pushed the editorial branch 2 times, most recently from 5c2afc7 to 3288957 Compare September 10, 2024 19:29
If running this test file without first importing the proposal polyfill,
it would fail due to the new intrinsics not being defined. We never
noticed this because we usually run datemath.mjs before this file.
In the calendar-specific code, in isoToCalendarDate(), we should not be
computing the era year if the calendar doesn't have eras.
In 'hebrew', 'chinese', and 'dangi' calendars, there are no eras. Any
eraYear property given in calendar fields must be ignored, as per
CalendarResolveFields in the spec text. Adjust the helpers for Hebrew and
Chinese/Dangi calendars accordingly; the eraYear was being used in place
of the year in some cases.

Closes: #2870
This was an overzealous replacement of CalendarDateAdd with BalanceISODate
spotted by Anba. It doesn't produce any observably different results,
because in all supported calendars weeks are always 7 days; but it's still
conceptually incorrect. Go back to using CalendarDateAdd here.

Closes: #2881
Otherwise the loop would not run.

Closes: #2890
As pointed out by Anba, this AO does nothing if smallestUnit equals
largestUnit, not just if smallestUnit equals "year". We can simplify the
early return condition.

See: #2890
smallestUnit is a date unit. In the loop, unitIndex runs from the index
before smallestUnit and is decremented, meaning that unit can only ever be
"year", "month", or "week".

Thanks to Anba for spotting this.

See: #2890
It's not possible for the duration to not be valid here, as _duration_ is
a valid duration, and the duration being tested is _duration_ truncated to
days, which can only ever be less than _duration_.

Spotted by Anba.

Closes: #2880
…an calendars

Dates in these calendars display an era when they are formatted in English
using Intl.DateTimeFormat, but not when they are formatted for the region
where the calendar is used. That likely indicates they shouldn't have
eras.

Remove `constantEra` from the calendar helpers, and introduce a new helper
that calculates dates relative to a fixed year zero.

Closes: #2899
As of the current state of the Intl Era and Month Codes proposal, the
eras table in https://tc39.es/proposal-intl-era-monthcode/#table-eras has
a canonical name for each era, and a number of aliases which should also
be accepted. Legacy ICU names such as "era0" should not be accepted,
although they still need to be handled when using Intl.DateTimeFormat to
convert dates.

Closes: #2901
The makeHelper___() functions already return a new object, so we don't
need to Object.assign() them to {}.
@ptomato ptomato merged commit 060c319 into main Sep 12, 2024
5 checks passed
@ptomato ptomato deleted the editorial branch September 12, 2024 16:22
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