fix(datetime): handle am / pm variants

[timreichen]

• allows for am / pm variants as described in http://www.unicode.org/reports/tr35/tr35-dates.html#Date_Field_Symbol_Table

[codecov]

Codecov Report

Attention: Patch coverage is 77.41935% with 7 lines in your changes missing coverage. Please review.

Project coverage is 96.24%. Comparing base (860f578) to head (9c93c1c).

Files	Patch %	Lines
datetime/_date_time_formatter.ts	77.41%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5406      +/-   ##
==========================================
- Coverage   96.26%   96.24%   -0.02%     
==========================================
  Files         473      473              
  Lines       38390    38419      +29     
  Branches     5571     5584      +13     
==========================================
+ Hits        36955    36978      +23     
- Misses       1393     1399       +6     
  Partials       42       42              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[iuioiua]

To my surprise, these tests all pass even without the implementation change. Why? I think we should add more thorough testing for dayPeriod before working on this addition. Also, we should update the format and parse documentation to reflect this change.

[babiabeo]

I think the problem is in partsToDate method:

std/datetime/_date_time_formatter.ts

Lines 642 to 649 in 3af2ed3

	case "hour": {
	let value = Number(part.value);
	const dayPeriod = parts.find(
	(part: DateTimeFormatPart) => part.type === "dayPeriod",
	);
	if (dayPeriod?.value === "PM") value += 12;
	utc ? date.setUTCHours(value) : date.setHours(value);
	break;

It only checks if dayPeriod.value is ”PM”. Otherwise, it ignores other values. That's why these tests all pass even without the implementation change.

[iuioiua]

@babiabeo, would you be interested in creating some tests to compliment this PR?

[iuioiua]

@timreichen, reminder that we can't merge this PR until the tests that it touches has been improved. Please see the above comments.

[timreichen]

Please hold with this PR until #5622.

[iuioiua]

@timreichen, have the tests been improved as requested above?

[timreichen]

@timreichen, have the tests been improved as requested above?

Yes, I added tests for all valid cases and a test that throws on invalid data.

[iuioiua]

Nice! LGTM.