Define date-time options used for date-range formatting as part of the date-range patterns. #25
Conversation
spec.emu
Outdated
| <li><ins>{[[Source]]: `"startRange"`, [[Pattern]]: `"{day}/{month}/{year}, {hour}:{minute}"`}</ins></li> | ||
| <li><ins>{[[Source]]: `"shared"`, [[Pattern]]: `" – "`}</ins></li> | ||
| <li><ins>{[[Source]]: `"endRange"`, [[Pattern]]: `"{hour}:{minute}"`}</ins></li> | ||
| <li><ins>[[year]]: `"2-digit"`</ins></li> |
There was a problem hiding this comment.
The field names should be in upper-case, because
- The description says they have the names of the internal slot names from http://tc39.es/proposal-intl-DateTimeFormat-formatRange/#table-datetimeformat-components, which are in upper-case.
- The modified http://tc39.es/proposal-intl-DateTimeFormat-formatRange/#sec-formatdatetimepattern algorithm also uses the internal slot names from http://tc39.es/proposal-intl-DateTimeFormat-formatRange/#table-datetimeformat-components.
If you want to use lower-case names for consistency with the existing format descriptions, you need to refer to the "Property" column instead of the "Internal Slot" column. But also see tc39/ecma402#81 and tc39/ecma402#339.
There was a problem hiding this comment.
Interesting, I wasn't aware of tc39/ecma402#81 and tc39/ecma402#339.
I prefer to keep it lower-case for the moment because updating it would require several updates in 1.1.1 InitializeDateTimeFormat (e.g. Line 28) as well as some parts in 1.3.3 Internal Slots, and I would prefer to minimize the number of changes unrelated to this proposal in order to keep the proposal's PR focused. But I totally agree that we should work on this on a follow-up spec fix.
Given this, I updated the 1.1.6 FormatDateTimePattern and 1.1.10 PartitionDateTimeRangePattern to reference to the property column instead. PTAL
spec.emu
Outdated
| @@ -294,7 +294,8 @@ contributors: Google, Ecma International | |||
| 1. Add new part record { [[Type]]: `"literal"`, [[Value]]: _literal_ } as a new element of the list _result_. | |||
| 1. Let _p_ be the substring of _pattern_ from position _beginIndex_, exclusive, to position _endIndex_, exclusive. | |||
| 1. If _p_ matches a Property column of the row in <emu-xref href="#table-datetimeformat-components"></emu-xref>, then | |||
| 1. Let _f_ be the value of _dateTimeFormat_'s internal slot whose name is the Internal Slot column of the matching row. | |||
| 1. <ins>If _rangeFormatOptions_ is not *undefined*, let _f_ be the value of _rangeFormatOptions_'s field whose name is the Internal Slot column of the matching row.</ins> | |||
There was a problem hiding this comment.
(Why doesn't GH allow me to annotate a line which wasn't modified in this PR? Sigh.)
Later one there's a step containing:
If p is "month", then the String value may also depend on whether dateTimeFormat has a [[Day]] internal slot. If p is "timeZoneName", then the String value may also depend on the value of the [[inDST]] field of tm. If p is "era", then the String value may also depend on whether dateTimeFormat has a [[Era]] internal slot and if the implementation does not have a localized representation of f, then use f itself.
Let's ignore that "has a [[Day]] internal slot" is a bug in ECMA-402 (internal slots are always present; this should instead check that the internal slot's value isn't undefined). That part probably now needs to include the case when rangeFormatOptions is used instead of dateTimeFormat.
(Hmm, I don't understand the "era" handling in that step. Specifically, "if the implementation does not have a localized representation of f, then use f itself", but f is one of "narrow", "short", or "long". And I'm pretty sure these strings shouldn't appear verbatim in the output...)
There was a problem hiding this comment.
Thanks for pointing this out! I had missed this. I updated this paragraph to account for rangeFormatOptions, PTAL.
Let's ignore that "has a [[Day]] internal slot" is a bug in ECMA-402 (internal slots are always present; this should instead check that the internal slot's value isn't undefined).
I updated this instance of this error.
(Hmm, I don't understand the "era" handling in that step. Specifically, "if the implementation does not have a localized representation of f, then use f itself", but f is one of "narrow", "short", or "long". And I'm pretty sure these strings shouldn't appear verbatim in the output...)
This sounds like a bug in the the spec, you are right that the value of f is not suitable (and doesn't make sense) for the output. I'll file a bug for this.
…e date-range patterns (Issue #21). Together with this, update FormatDateTimePattern to use these options instead of the default options used by date-time formatting.
|
I rebased latest changes in in the main branch. @anba PTAL, I'll merge this PR as soon as you LGTM. Thanks! |
Co-authored-by: Leo Balter <[email protected]>
Fixes #21.