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

Editorial: Add note to ref to UTS35 about weekinfo #87

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

FrankYFTang
Copy link
Collaborator

Address #30 by deferring to UTS35
and invalidate tc39/ecma402#950

@ben-allen
Copy link

Making sure I understand the broader matter correctly: the current wording will respect -u-ca, but because "rg" is not in [[RelevantExtensionKeys]], -u-rg will not be taken into account when determining the returned Record's value?

I wonder if this note on the whole might be clearer if the specific extensions that affect the Record's value were listed in the note, and if it were explicitly stated that https://www.unicode.org/reports/tr35/tr35-dates.html#first-day-overrides determines which of the components of _locale_ have priority.

@anba
Copy link
Contributor

anba commented Aug 5, 2024

[[RelevantExtensionKeys]] for Intl.Locale doesn't have the same semantics as [[RelevantExtensionKeys]] for Intl service constructors. I guess it's now finally time to rename [[RelevantExtensionKeys]] for Intl.Localetc39/ecma402#914.

@FrankYFTang
Copy link
Collaborator Author

My intention of this note is to delegate the whole algorithm to UTS35 instead of specify a half baked algorithm that we need to keep updating the algorithm if UTS35 change later.

@@ -203,6 +203,10 @@ <h1>WeekInfoOfLocale ( _loc_ )</h1>
1. Return _r_.
</emu-alg>

<emu-note>
The record's return values are determined by _locale_, in accordance with the specifications outlined in <a href="https://www.unicode.org/reports/tr35/tr35-dates.html#Week_Data">UTS 35's Week Data</a> and <a href="https://www.unicode.org/reports/tr35/tr35-dates.html#first-day-overrides">First Day Overrides</a>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???? do you have internet problem? it is a good link

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The anchor "#first-day-overrides" doesn't exist.

The relevant source code:

<h4>First Day Overrides</h4>

This must instead be:

<h4><a name="first-day-overrides" href="#first-day-overrides">First Day Overrides</a></h4>

for anchors to work.

Copy link
Collaborator Author

@FrankYFTang FrankYFTang Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But

<script src="./js/anchor.min.js"></script>

in the beginning of https://www.unicode.org/reports/tr35/tr35-dates.html add the id for the <h4>

see https://www.w3schools.com/html/html_id.asp
and https://www.bryanbraun.com/anchorjs/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks, that's explains it. With JS disabled, the "#first-day-overrides" anchor doesn't work, whereas all other anchors I've tried do work.

@anba
Copy link
Contributor

anba commented Aug 8, 2024

UTS 35 has validity requirements which are not implemented in ICU4C.

Relevant specifications for rg and sd, including their validity requirements:

UTS 35 contains examples for validity checks in the "Unit preference overrides" section:

Is it a normative requirement to implement the validity checks when referencing UTS 35's "First Day Overrides"?


Results for some locales and the returned week info objects from the V8 implementation:

Locale Week Info Correctness Comment
en {firstDay: 7, weekend: [6, 7], minimalDays: 1} Correct Missing region, add US per likely-subtags.
en-US {firstDay: 7, weekend: [6, 7], minimalDays: 1} Correct Expected week info for US.
en-DE {firstDay: 1, weekend: [6, 7], minimalDays: 4} Correct Expected week info for DE.
en-DE-u-rg-uszzzz {firstDay: 7, weekend: [6, 7], minimalDays: 1} Correct Region override to use US.
en-DE-u-rg-aazzzz {firstDay: 1, weekend: [6, 7], minimalDays: 1} Incorrect Region AA doesn't exist, so region override is invalid and should be ignored per UTS 35.
en-DE-u-rg-001zzz {firstDay: 1, weekend: [6, 7], minimalDays: 1} Incorrect 001 is a macro region, so region override is invalid and should be ignored per UTS 35.

@FrankYFTang
Copy link
Collaborator Author

FrankYFTang commented Aug 8, 2024

UTS 35 has validity requirements which are not implemented in ICU4C.

Relevant specifications for rg and sd, including their validity requirements:

UTS 35 contains examples for validity checks in the "Unit preference overrides" section:

Is it a normative requirement to implement the validity checks when referencing UTS 35's "First Day Overrides"?

Results for some locales and the returned week info objects from the V8 implementation:

v8 is on ICU 74-2 and not updated to ICU 75-1 yet. Some of the UTS35 is implemented in 75-1. I need to check are the two issues you mentioned due to 74-2 vs 75-1 or the trunk is not correct yet.

@FrankYFTang
Copy link
Collaborator Author

UTS 35 has validity requirements which are not implemented in ICU4C.

could you file an ICU ticket about what is missing? I am not quite sure I understand what is missing there now. Comparing the current v8 behavior is not a good idea since I added some of that latest UTS35 support into 75-1 but v8 is still on 74-2 so far.

Copy link

@ben-allen ben-allen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like putting this in a note, since it's required rather than recommended behaviour. How about defining Step 3 as something like the following:

  1. Let r be a Record with fields as defined by Table 2, with field values calculated from locale per the specifications in UTS 35's Week Data and the algorithm given in First Day Overrides.

on edit: I believe the suggested text would make this a normative change rather than an editorial change.

@anba
Copy link
Contributor

anba commented Aug 12, 2024

UTS 35 has validity requirements which are not implemented in ICU4C.

could you file an ICU ticket about what is missing? I am not quite sure I understand what is missing there now.

Sure. Filed as https://unicode-org.atlassian.net/browse/ICU-22854.

@anba
Copy link
Contributor

anba commented Aug 21, 2024

Hmm, I just saw this in https://tc39.es/ecma402/#sec-language-tags (emphasis mine):

All structurally valid language tags are appropriate for use with the APIs defined by this specification, but implementations are not required to use Unicode Common Locale Data Repository (CLDR) data for validating them;

This looks like another problem when directly referencing to UTS 35 for week info processing, because UTS 35 does require validation. Maybe you can sidestep this issue by adding a note that validation as performed in UTS 35 is not required for ECMA-402?

Copy link
Collaborator Author

@FrankYFTang FrankYFTang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@FrankYFTang FrankYFTang merged commit 091f59e into main Nov 21, 2024
@FrankYFTang FrankYFTang deleted the AddNoteOfWeekData branch November 21, 2024 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants