Add support for week-of-month. #1468

mildgravitas · 2022-01-04T12:18:56Z

Fixes #502
Fixes #488

Contrary to UTS 35 this always uses min_days = 1 as there's no month of
week-of-month field so there would be inconsistencies otherwise (e.g. in
the ISO calendar 2021-01-01 is the last week of December but 'MMMMW' would
have it formatted as 'week 5 of January').

nordzilla · 2022-01-05T17:41:35Z

components/datetime/src/date.rs

+    first_weekday: IsoWeekday,
+) -> Result<WeekOfMonth, DateTimeError> {
+    // This value isn't used by week_of() given that we use a calendar with min_week_days = 1.
+    const UNUSED_DAYS_IN_MONTH: u16 = 60;


Thought

This seems a little bit strange to me. But I'm not familiar enough with this code to know what alternatives would be.

As someone who is not familiar with the week_of code, I'm not sure why the min_week_days = 1 is relevant, or why this value is 60.

Question

Is the 60 an arbitrary value, since this is "unused," or does this value have to be 60?

Question

I wonder if we can't do something a bit cleaner using Option.

Could week_of::week_of potentially take an Option?

My bad, I was leaking arithmetic.rs's implementation details there. I've added a simple_week_of() method to arithmetic.rs that encapsulates those details instead. If that's still non-ideal I can add an implementation for t simple_week_of() rather than call into the more complete week_of(); I'd rather avoid having Options complicate the week_of signature...

With min_week_days = 1 week_of() boils down to ceil((day - 1 + (first_day_of_month - first_weekday)) / 7). num_days_in_previous_unit & num_days_in_unit only come into play for min_week_days > 1 in which case a day in the current month/year can get counted as part of the previous or next month/year.

jira-pull-request-webhook · 2022-01-07T15:15:26Z

Notice: the branch changed across the force-push!

provider/testdata/data/testdata.postcard is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

gregtatum

Thanks, looks good to me. 👍

components/datetime/tests/fixtures/tests/components-exact-matches.json

components/datetime/src/skeleton/error.rs

components/datetime/src/options/components.rs

components/datetime/src/date.rs

jira-pull-request-webhook · 2022-01-27T16:48:48Z

Notice: the branch changed across the force-push!

components/datetime/src/date.rs is different
components/datetime/src/options/components.rs is different
components/datetime/src/pattern/runtime/plural.rs is different
components/datetime/src/skeleton/mod.rs is different
components/datetime/tests/fixtures/tests/components-partial-matches.json is now changed in the branch
provider/testdata/data/testdata.postcard is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

zbraniecki

lgtm!

sffc · 2022-02-17T18:27:40Z

@mildgravitas Can you update the postcard file again, and then merge? You should have merge access now.

jira-pull-request-webhook · 2022-02-21T12:32:06Z

Notice: the branch changed across the force-push!

components/datetime/src/fields/symbols.rs is different
components/datetime/src/format/datetime.rs is different
components/datetime/src/pattern/runtime/plural.rs is different
components/datetime/src/skeleton/mod.rs is different
provider/testdata/data/testdata.postcard is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

jira-pull-request-webhook · 2022-03-16T12:08:12Z

Notice: the branch changed across the force-push!

components/calendar/src/arithmetic.rs is different
components/datetime/src/date.rs is different
components/datetime/src/fields/symbols.rs is different
components/datetime/src/format/datetime.rs is different
components/datetime/src/skeleton/error.rs is different
components/datetime/src/skeleton/mod.rs is different
components/datetime/tests/fixtures/tests/components-exact-matches.json is different
components/datetime/tests/fixtures/tests/components-partial-matches.json is different
provider/testdata/data/testdata.postcard is no longer changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

Contrary to UTS 35 this always uses min_days = 1 as there's no month of week-of-month field so there would be inconsistencies otherwise (e.g. in the ISO calendar 2021-01-01 is the last week of December but 'MMMMW' would have it formatted as 'week 5 of January').

…s into calendar.

jira-pull-request-webhook · 2022-03-16T12:12:37Z

Notice: the branch changed across the force-push!

provider/testdata/data/testdata.postcard is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

mildgravitas · 2022-03-16T13:15:38Z

Rebased onto 0fd8cde. I had to update components-exact-matches.json too from #1366. PTAL.

sffc · 2022-03-19T02:17:57Z

@mildgravitas Since @gregtatum is on PTO, feel free to hit the merge button!

gregtatum

Thanks for the rebase!

mildgravitas marked this pull request as ready for review January 4, 2022 12:46

mildgravitas requested review from gregtatum, nordzilla, zbraniecki and a team as code owners January 4, 2022 12:46

nordzilla reviewed Jan 5, 2022

View reviewed changes

mildgravitas force-pushed the week_of_month branch from f0fac1d to 9a10562 Compare January 7, 2022 15:15

gregtatum previously approved these changes Jan 14, 2022

View reviewed changes

mildgravitas dismissed gregtatum’s stale review via aa23afd January 27, 2022 16:48

mildgravitas force-pushed the week_of_month branch from 9a10562 to aa23afd Compare January 27, 2022 16:48

zbraniecki previously approved these changes Feb 3, 2022

View reviewed changes

mildgravitas dismissed zbraniecki’s stale review via 5d798fc February 21, 2022 12:32

mildgravitas force-pushed the week_of_month branch from aa23afd to 5d798fc Compare February 21, 2022 12:32

sffc assigned gregtatum Mar 3, 2022

sffc added this to the 2022 Q1 0.6 Sprint D milestone Mar 3, 2022

mildgravitas force-pushed the week_of_month branch from 5d798fc to 586bc5b Compare March 16, 2022 12:08

mildgravitas added 4 commits March 16, 2022 13:12

refactor(datetime,calendar): Move week_of_month implementation detail…

874e818

…s into calendar.

tests(datetime): add a partial match test for week-of-month

3edf127

docs(datetime): update & expand week-of docstrings

a7eb37a

mildgravitas force-pushed the week_of_month branch from 586bc5b to a7eb37a Compare March 16, 2022 12:12

mildgravitas requested a review from gregtatum March 16, 2022 13:16

sffc requested a review from nordzilla March 18, 2022 17:49

nordzilla approved these changes Mar 18, 2022

View reviewed changes

gregtatum approved these changes Mar 21, 2022

View reviewed changes

gregtatum merged commit d3cbc28 into unicode-org:main Mar 21, 2022

mildgravitas deleted the week_of_month branch March 23, 2022 11:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for week-of-month. #1468

Add support for week-of-month. #1468

mildgravitas commented Jan 4, 2022 •

edited

Loading

nordzilla Jan 5, 2022

mildgravitas Jan 7, 2022

mildgravitas Jan 7, 2022

jira-pull-request-webhook bot commented Jan 7, 2022

gregtatum left a comment

jira-pull-request-webhook bot commented Jan 27, 2022

zbraniecki left a comment

sffc commented Feb 17, 2022

jira-pull-request-webhook bot commented Feb 21, 2022

jira-pull-request-webhook bot commented Mar 16, 2022

jira-pull-request-webhook bot commented Mar 16, 2022

mildgravitas commented Mar 16, 2022

sffc commented Mar 19, 2022

gregtatum left a comment

Add support for week-of-month. #1468

Add support for week-of-month. #1468

Conversation

mildgravitas commented Jan 4, 2022 • edited Loading

nordzilla Jan 5, 2022

Choose a reason for hiding this comment

mildgravitas Jan 7, 2022

Choose a reason for hiding this comment

mildgravitas Jan 7, 2022

Choose a reason for hiding this comment

jira-pull-request-webhook bot commented Jan 7, 2022

gregtatum left a comment

Choose a reason for hiding this comment

jira-pull-request-webhook bot commented Jan 27, 2022

zbraniecki left a comment

Choose a reason for hiding this comment

sffc commented Feb 17, 2022

jira-pull-request-webhook bot commented Feb 21, 2022

jira-pull-request-webhook bot commented Mar 16, 2022

jira-pull-request-webhook bot commented Mar 16, 2022

mildgravitas commented Mar 16, 2022

sffc commented Mar 19, 2022

gregtatum left a comment

Choose a reason for hiding this comment

mildgravitas commented Jan 4, 2022 •

edited

Loading