Editorial: Refactor the last bit of prose in NonISOCalendarDateToISO to AOs

[sffc]

I was complimented by @gibson042 for making smaller PRs with pieces of #69. Here is the Thursday/Friday edition.

[sffc]

Note: this spec works by interpreting day as an ordinal day. I don't think we use any other type of day. I think this AO reads as though @ptomato had thought that day should be the day number as opposed to an ordinal day. I'm open to using language such as ordinalDay to make my interpretation more clear.

[sffc]

Note the definition of [[Day]]:

          <tr>
            <td>[[Day]]</td>
            <td>a positive integer</td>
            <td>
              The 1-based ordinal position of the date's day within its month.
            </td>
          </tr>

It already says it is the ordinal day, not the day number.

Should we still be more clear?

The edge case, of course, are dates such as October 10, 1867 that don't exist in certain regions like Alaska due to Gregorian switchover. If you write code such as

Temporal.PlainDate.from({ calendar: "gregory-usak", year: 1867, month: 10, day: 10 })

should it:

1. Constrain down to October 5, 1867
2. Constrain up to October 18, 1867
3. Interpret the day as an ordinal and return October 22, 1867 (the 10th day in October that year)

Option 3 is much easier implementation-wise than anything else.

This is also more of an academic question at this point because we don't have julian-gregorian support.

[ptomato]

Oh, I see your point now about this looking like it refers to non-ordinal days. I intended it to refer to days that ran off the end of a month, like February 30. I got the language from NonISOCalendarDateToISO and probably "If there are two equally-close dates in that month, pick the later one" should be removed from there. I would guess that was written before we defined day as ordinal day (which happened in a normative change in January 2023)

[sffc]

Also, in CalendarYearMonthFromFields:

2. Let firstDayIndex be the 1-based index of the first day of the month described by fields (i.e., 1 unless the month's first day is skipped by this calendar.)

[sffc]

I realized that this RangeError isn't in the prose being edited, but I think it was wrongfully removed in #73. Previously the spec said

        1. If _fields_ describes an existing date in _calendar_, return an implementation-defined ISO Date Record that corresponds to the date described by _fields_.
        1. If _overflow_ is ~reject~, throw a *RangeError* exception.

So it would have thrown on an invalid day.

[ptomato]

I don't think we can get here - Calendar Fields Records can only have a positive integer or UNSET in the [[Day]] field.

[sffc]

Done, good catch

[ptomato]

I'm not sure this operation adds much when pulled out into its own AO, and everywhere else we use year/month code/day to refer unambiguously to a date, not the ordinal month. (Same thing for CalendarDaysInMonth.) I don't see this operation on the ICU4X flowchart either? Should it conceptually be part of ICU4X and not Temporal?
I guess you probably have some purpose in mind for it.

If you decide to keep it I'd probably look for a different name. I find the term "calendar integers" confusing, how about NonISOCalendarDateWithOrdinalMonthToISO?

[sffc]

This AO makes more sense in the next PR but I can hold it for then if you prefer

[ptomato]

Up to you.

[sffc]

I think this is valuable even in the context of the current PR because it makes the calendar-specific algorithm be isolated to a small AO.