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

Reorganize PrepareTemporalFields and CalendarFields abstract operations #2630

Closed
ptomato opened this issue Jul 18, 2023 · 1 comment · Fixed by #2823
Closed

Reorganize PrepareTemporalFields and CalendarFields abstract operations #2630

ptomato opened this issue Jul 18, 2023 · 1 comment · Fixed by #2823
Assignees
Labels
editorial spec-text Specification text involved
Milestone

Comments

@ptomato
Copy link
Collaborator

ptomato commented Jul 18, 2023

Make the remaining editorial refactor described in this comment in #2570:

  • We'd like to throw on duplicates returned from CalendarFields, while still preserving the fact that it's not necessarily a bug if the two invocations of fields() return two unique lists that have some overlap when merged.
  • Every invocation of CalendarFields is always followed by PrepareTemporalFields with no observable operation in between, so it doesn't matter if we throw on duplicates at the end of CalendarFields, or after sorting at the beginning of PrepareTemporalFields. Anba has indicated that throwing after sorting in PrepareTemporalFields allows optimization by only iterating the list once.
  • The problem with throwing on duplicates in PrepareTemporalFields is that although CalendarFields is always followed by PrepareTemporalFields in the current spec, there's no guarantee that CalendarFields will continue to be used correctly in the future. Replace CalendarFields with two new AOs: a simple one that returns the result of PrepareTemporalFields, and a complex one that returns a record with both the names (the result of fields()) and the object result of PrepareTemporalFields. This new AO would need to accept an additional List of names to be added, e.g. *offset* and *timeZone*. If adding those values creates duplicates, then throw.

Originally posted by @ptomato in #2570 (comment)

@justingrant
Copy link
Collaborator

Meeting 2023-08-17: Philip will get to this after previously-approved normative changes are landed.

@ptomato ptomato added this to the Stage "3.5" milestone Feb 1, 2024
@ptomato ptomato added the spec-text Specification text involved label Feb 1, 2024
ptomato added a commit that referenced this issue Apr 18, 2024
…elds

This adds a new operation PrepareCalendarFields which consists of calling
the `fields` method of a calendar with a list of calendar property names,
appending a list of non-calendar property names to the result, followed by
calling PrepareTemporalFields to read all of the given properties from the
given object.

See: #2630
ptomato added a commit that referenced this issue Apr 18, 2024
This removes the remaining uses of CalendarFields by refactoring them into
PrepareCalendarFieldsAndFieldNames, which does the same thing as
PrepareCalendarFields but also returns the field names obtained from the
calendar, as well as the object returned from PrepareTemporalFields.

After this commit, it is no longer possible to call CalendarFields without
validating that there are no duplicates in the array returned from the
calendar's fields() method.

Closes: #2630
Ms2ger pushed a commit that referenced this issue Apr 26, 2024
…elds

This adds a new operation PrepareCalendarFields which consists of calling
the `fields` method of a calendar with a list of calendar property names,
appending a list of non-calendar property names to the result, followed by
calling PrepareTemporalFields to read all of the given properties from the
given object.

See: #2630
Ms2ger pushed a commit that referenced this issue Apr 26, 2024
This removes the remaining uses of CalendarFields by refactoring them into
PrepareCalendarFieldsAndFieldNames, which does the same thing as
PrepareCalendarFields but also returns the field names obtained from the
calendar, as well as the object returned from PrepareTemporalFields.

After this commit, it is no longer possible to call CalendarFields without
validating that there are no duplicates in the array returned from the
calendar's fields() method.

Closes: #2630
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial spec-text Specification text involved
Projects
None yet
2 participants