tc39 / proposal-intl-locale-info

An API to expose information of locale, such as week data ( first day of a week, weekend start, weekend end), hour cycle, measurement system, commonly used calendar, etc.
MIT License
56 stars 11 forks source link

Review notes #26

Closed anba closed 3 years ago

anba commented 3 years ago

Also some quick review notes for this proposal:


  1. CreateArrayFromListAndPreferred()
    • Can be simplified to either:
      1. If preferred is undefined, then
        1. Return ! CreateArrayFromList(list).
      2. Return ! CreateArrayFromList(« preferred »).
  1. CalendarsOfLocale ( loc )

    • Add requirement that the values are in lower-case.
    • Add requirement that no duplicates are returned.
    • Prefix CreateArrayFromListAndPreferred with "!".
    • Also applies to all other sections.
  2. CalendarsOfLocale ( loc )

  3. CalendarsOfLocale ( loc )

    • I think "in common use in the locale" should be "in common use in locale", so without the "the".
    • Also I wonder if "for date and time formatting" should appear directly after "in common use".
    • Please check both points with native speakers.
    • (Same issue also applies to other sections.)
  4. TimeZonesOfLocale ( loc )

    • Any reason why the returned names aren't canonicalised?
    • If the names are canonicalised, "Link names" can't be appear here, so step 4 should be reworded accordingly.
    • Define behaviour when region is an unsupported region.
    • For example what is the return value of TimeZonesOfLocale("und-ZX"), where "ZX" isn't a registered region subtag, or what is returned for TimeZonesOfLocale("und-ZZ"), where "ZZ" is the unknown region placeholder, also what should be returned for larger regions like TimeZonesOfLocale("und-019"), where "019" is the region subtag for the Americas?
  5. get Intl.Locale.prototype.textInfo

  6. get Intl.Locale.prototype.weekInfo

    • Same issue about moving the locale-specific computation into a separate abstract operation as mentioned for textInfo.
    • This separate abstract operation could be defined to return a record {[[FirstDay]], [[WeekendStart]], [[WeekendEnd]], [[MinimalDays]]}, which is then processed in weekInfo.
anba commented 3 years ago

Additional notes:

(*) If I interpret https://github.com/unicode-org/icu/blob/main/icu4c/source/tools/tzcode/tz2icu.cpp correctly, the newer file version from https://github.com/eggert/tz/blob/main/zone1970.tab isn't used.

anba commented 3 years ago

This recently proposed patch for tzdata could also be of interest: https://mm.icann.org/pipermail/tz/2021-May/030078.html.

It changes multiple time zones to be links when there's an equivalent zone which has matching time zones rules after 1970. For example Europe/Amsterdam is changed to a link to Europe/Brussels in the "backward" file. (The historic rules for Europe/Amsterdam will still be available in the "backzone" file.) And because Europe/Amsterdam is now a link to Europe/Brussels, both "zone.tab" and "zone1970.tab" then define Europe/Brussels as the time zone for "NL" (the Netherlands).

So new Intl.Locale("nl-NL").timeZones will change from ["Europe/Amsterdam"] to ["Europe/Brussels"], if that patch is accepted.

The already applied patch https://github.com/eggert/tz/commit/59dda9ec3e5eb5b01b45716dc5802c490022f9b3 has similar effects for "zone.tab". ("zone1970.tab" already omitted links). For example new Intl.Locale("und-BA").timeZones and new Intl.Locale("und-HR").timeZones will soon (*) return ["Europe/Belgrade"]. So we'll likely run into the same negative cultural/political issues mentioned in https://github.com/tc39/ecma402/issues/272.

(*) Unless ICU decides to add manual overrides for these time zones.

Apropos negative cultural/political issues, when directly using the output from ICU, "Europe/Simferopol" is only listed for new Intl.Locale("und-UA").timeZones, but not for new Intl.Locale("und-RU").timeZones. Also compare https://github.com/eggert/tz/blob/a7166005212fc5ba05f9553bfd65b679fb8ae953/zone1970.tab#L292 and https://github.com/eggert/tz/blob/a7166005212fc5ba05f9553bfd65b679fb8ae953/zone.tab#L338-L341. And maybe https://unicode-org.atlassian.net/browse/ICU-12802 and https://unicode-org.atlassian.net/browse/CLDR-9816, where the override was added to ICU.

FrankYFTang commented 3 years ago

@anba Really appreciate your careful review. Since this is already in Stage 3, let me first address easy Editorial changes suggested by you.

Please see #34 for item 1-4 and see #35 for item 6-7. I will think about how to address item 5 and others a little bit more after we land these two PRs.

anba commented 3 years ago

This recently proposed patch for tzdata could also be of interest: https://mm.icann.org/pipermail/tz/2021-May/030078.html.

The corresponding patch landed yesterday: https://github.com/eggert/tz/commit/1edbb16e933a6ba6dceefd2bd7057b5ce00dd13c

FrankYFTang commented 3 years ago

I add a new issue to track item3 separately https://github.com/tc39/proposal-intl-locale-info/issues/38

FrankYFTang commented 3 years ago

I feel this issue is being hard to track. I would like to close this issue and if you still have items I have not addressed of what you mentioned above could you file indivisual issue so we can track and address one by one?

anba commented 3 years ago

Sure. I've filed #45, #46, and #47 for the remaining open issues.