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.

Sign up for GitHub

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

Jump to bottom

Digital duration format should probably be more lenient with data #161

Closed
sffc opened this issue Jul 24, 2023 · 5 comments · Fixed by #188
Closed

Digital duration format should probably be more lenient with data #161

sffc opened this issue Jul 24, 2023 · 5 comments · Fixed by #188
Assignees
@ben-allen @ryzokuken

Comments

@sffc
Copy link
Collaborator

sffc commented Jul 24, 2023

The spec currently uses a fixed separator for the digital format:

i. Let numberingSystem be durationFormat.[[NumberingSystem]].
ii. Let separator be dataLocaleData.[[digitalFormat]].[[<numberingSystem>]].
iii. Append the new [Record](https://tc39.es/ecma262/#sec-list-and-record-specification-type) { [[Type]]: "literal", [[Value]]: separator} to the end of last.

This works in most cases, but there are some locales that have data that potentially breaks from these assumptions:

  • af: <durationUnitPattern>hh:mm</durationUnitPattern> (prefers 2-digit hour)
  • ee: <durationUnitPattern draft="unconfirmed">aɖabaƒoƒo m:ss</durationUnitPattern> (has some other stuff in the pattern)
  • nds: <durationUnitPattern draft="unconfirmed">h.mm:ss</durationUnitPattern> (note the different hour and minute separator)

The CLDR digital duration data looks like this:

https://github.com/unicode-org/cldr/blob/706d94d95c855bb281a1829488c511f99e689ecf/common/main/af.xml#L9216

Note that two of the three above cases are "unconfirmed", and the third is only about a preference for 2-digit hours, which we could potentially support even if we don't support generalized patterns.
@FrankYFTang
Copy link
Collaborator

FrankYFTang commented Jul 24, 2023 •
edited

  1. I do not believe currently ICU take draft="unconfirmed" data
  2. Looks it is in ICU under
    source/data/unit/*.txt/durationUnits/(hm|hms|ms)

It seems as ICU 73-1,
A. we have three possible values for durationUnits/hms
h:mm:ss
hh:mm:ss
h.mm.ss

B. we have possible 3 values for durationUnits/hm
h:mm
hh:mm
h.mm

C. we have possible 3 values for durationUnits/ms
m:ss
mm:ss
m.ss

@FrankYFTang
Copy link
Collaborator

FrankYFTang commented Jul 24, 2023 •
edited

So... there are actually three different issues here

  1. the character used for the separator
  2. while the callers does not specify 2-digits or numeric for each specific units and the style is digit, to use 2-digits or numeric should come from locale data instead of always a specific value specified in the algorithm.
  3. what should we do if the caller specify 2-digits or numeric for each specific units but it does not agree w/ the locale data in pattern?

@sffc
Copy link
Collaborator Author

sffc commented Aug 23, 2023

@ben-allen or @ryzokuken please consider drafting a spec change to fix this. This is a bug fix, even if it could result in normative changes.

@sffc
Copy link
Collaborator Author

sffc commented Nov 16, 2023

Discussed in TG2 call: https://github.com/tc39/ecma402/blob/master/meetings/notes-2023-11-16.md#digital-duration-format-should-probably-be-more-lenient-with-data-161

Conclusion: Ideally this would have been ready for the November 2023 TC39 meeting, but if it needs to slip it's okay. We should try to get it in soon though because it blocks implementations.

@ben-allen ben-allen mentioned this issue Mar 7, 2024
Merged
@ben-allen
Copy link
Collaborator

#188 addresses most of this issue by

  1. providing separate [[HoursMinutesSeparator]] and [[MinutesSecondsSeparator]] slots
  2. Providing a [[TwoHourDigits]] slot to indicate when locales prefer always displaying two digit hours in digital formats

These address, respectively, the issue in the 'nds' locale (and in other locales which also use differing hour/minute and minute/second separators) and the issue in the 'af' locale. I'm comfortable leaving the 'ee' problem unaddressed unless/until there's confirmation that the CLDR data for this locale is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ben-allen ben-allen

@ryzokuken ryzokuken

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@sffc @ben-allen @ryzokuken @FrankYFTang