Modify patterns from a skeleton match to have the correct width

[gregtatum]

Resolves #584.

[coveralls]

Pull Request Test Coverage Report for Build a51364a4f74669190872dd6e0ea8e3bd91dac4bb-PR-832

• 20 of 23 (86.96%) changed or added relevant lines in 1 file are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.06%) to 74.938%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
components/datetime/src/skeleton.rs	20	23	86.96%

Files with Coverage Reduction	New Missed Lines	%
components/datetime/src/skeleton.rs	2	82.04%

Totals
Change from base Build 67bd340dd7cb6e1a958ceb36f8f1d4e73c63742e:	0.06%
Covered Lines:	9694
Relevant Lines:	12936

---

💛 - Coveralls

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 86.95652% with 3 lines in your changes missing coverage. Please review.

Project coverage is 74.86%. Comparing base (67bd340) to head (2ba7ad7).
Report is 3733 commits behind head on main.

Files with missing lines	Patch %	Lines
components/datetime/src/skeleton.rs	86.95%	3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #832      +/-   ##
==========================================
+ Coverage   74.80%   74.86%   +0.06%     
==========================================
  Files         198      198              
  Lines       12762    12778      +16     
==========================================
+ Hits         9546     9566      +20     
+ Misses       3216     3212       -4     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sffc]

Not really requesting changes, but I have a question before I approve

[sffc]

Thought: I wish we could do this without alloc. I liked how before this function was basically a projection from &'a SkeletonsV1 to &'a Pattern. The extra allocation is going to hurt basically every benchmark we're after: memory usage, code size, and performance.

Suggestion: Could this be architected in a way that involves a map on the pattern itself? Like, you could attach a mapping function to the pattern, such that when you loop over the pattern while formatting, you run the PatternItems through the mapping function.

Alternatively, is this in the part of the code that we won't use at runtime after the new CLDR 70 skeleton algorithm is implemented? I don't care about the Vec if it's run in the transformer.

[gregtatum]

The extra allocation is going to hurt basically every benchmark we're after.

Yes, I agree that the allocation isn't great, and what I tried to design around in the initial implementation.

Suggestion: Could this be architected in a way that involves a map on the pattern itself.

The pattern must be mutated in order to support all of the features for DateTimeFormat. Beyond just the widths, there's also the hour cycle to consider. There's also append items support for things like time zones or week. The latter is even trickier, as a simple mapping function wouldn't work.

I would prefer to accept the perf/memory regression for 0.3, and follow-up with exploring some approaches. Especially as pattern mutation is a new requirement for other features such as the hour cycle, and append items. Then for 0.4 I can track fixing it.

Alternatively, is this in the part of the code that we won't use at runtime after the new CLDR 70 skeleton algorithm is implemented? I don't care about the Vec if it's run in the transformer.

I don't think well want a literal pattern for every width adjustment, as it will greatly affect the provider data. I'd say let's follow-up with looking for perf wins here. I'm nervous about the trade off between runtime characteristics and data payload size. I think it's something we should definitely try and work out.

[gregtatum]

I filed #877 as a follow-up, if you are OK with moving forward with this patch.

[sffc]

Okay. I won't block this PR but I'm a bit disappointed that we're going in this direction. I feel like we're repeating the problems of ICU4C rather than solving them. I hope we can follow up in #877.

[gregtatum]

Well, this at least gets us a baseline metric that we can track, and tests in place that make sure we are compliant with the behavior. The code is mutable, so we can course correct to fewer allocations. I also filed #879 to make sure we track this area better. I realize now that the benchmark is still needed.

Follow-up issue