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

Make firstDayOfWeek return string and fix spec bug #79

Merged
merged 4 commits into from
Jan 2, 2024
Merged

Conversation

FrankYFTang
Copy link
Collaborator

@FrankYFTang FrankYFTang commented Nov 16, 2023

This revert our change after TC39 Sept meeting so

I. Intl.Locale.prototype.firstDayOfWeek returns any strings which fit type = alphanum{3,8} (sep alphanum{3,8})*. as defined in UTS35
2. {firstDayOfWeek} for new Intl.Locale() accepts: strings fit Default: undefined

@FrankYFTang
Copy link
Collaborator Author

Sorry. I just realize this PR is not good enough. Need more work. I need to rework on this. Intl.Locale.prototype.firstDayOfWeek need to return string, but not just that 7 string values.

@FrankYFTang
Copy link
Collaborator Author

  • Consensus reach to merge PR 79 to align other Intl.Locale API in TC39 2023-11-28 meeting

@FrankYFTang
Copy link
Collaborator Author

@sffc @anba @ben-allen ping

@FrankYFTang FrankYFTang merged commit 8b3832e into main Jan 2, 2024
@FrankYFTang FrankYFTang deleted the fixFW branch January 2, 2024 21:41
@trflynn89
Copy link

Hey @FrankYFTang - While implementing this, I came across a small issue. I think WeekInfoOfLocale needs to be updated to match the [[FirstDayOfWeek]] changes. [[FirstDayOfWeek]] can now be an arbitrary string, but steps 4-5 of WeekInfoOfLocale state:

4. Let fw be loc.[[FirstDayOfWeek]].
5. If fw is not undefined, then
    a. Set r.[[FirstDay]] to fw.

Where r.[[FirstDay]] is an "Integer value between 1 and 7". So I think there needs to be a step to convert fw from a string to an integer here.

@sosukesuzuki
Copy link

@FrankYFTang @ptomato Maybe we should update test262 for this?

@ptomato
Copy link

ptomato commented Jul 9, 2024

It's not clear to me what changed just from looking at the PR, but yes please do update test262 if it's needed.

@FrankYFTang
Copy link
Collaborator Author

Hey @FrankYFTang - While implementing this, I came across a small issue. I think WeekInfoOfLocale needs to be updated to match the [[FirstDayOfWeek]] changes. [[FirstDayOfWeek]] can now be an arbitrary string, but steps 4-5 of WeekInfoOfLocale state:

4. Let fw be loc.[[FirstDayOfWeek]].
5. If fw is not undefined, then
    a. Set r.[[FirstDay]] to fw.

Where r.[[FirstDay]] is an "Integer value between 1 and 7". So I think there needs to be a step to convert fw from a string to an integer here.

fork that issue into #82

FrankYFTang added a commit to FrankYFTang/test262 that referenced this pull request Jul 9, 2024
ptomato pushed a commit to FrankYFTang/test262 that referenced this pull request Jul 10, 2024
ptomato pushed a commit to tc39/test262 that referenced this pull request Jul 10, 2024
hubot pushed a commit to v8/v8 that referenced this pull request Jul 11, 2024
Update the implementation to sync with
https://tc39.es/proposal-intl-locale-info/

Change to reflect
tc39/proposal-intl-locale-info#79

Bug: 42204095
Change-Id: Ie0aeb104499e3e7e600e1ab2b0c54ac112ff7930
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5690278
Reviewed-by: Frank Tang <ftang@chromium.org>
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#94950}
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