Components bag support with only skeleton matching #587
Conversation
|
|
||
| fields | ||
| } | ||
| } |
There was a problem hiding this comment.
Thought (future): Perhaps this logic should live in the ecma402 crate. We can design a simpler components bag API for the actual ICU4X crate, and leave this mapping logic up to just the 402 compat layer.
gregtatum
force-pushed
the
components-bag
branch
from
390a695 to
361c375
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
|
There's been a bunch of conflicting code landed in the last week, so I opted for a rebase to handle rather than merge as it was much simpler to verify the correctness in order to handle the conflicts. This is still ready for re-review. |
|
|
||
| // Shorten these for terser tests. | ||
| type FS = FieldSymbol; | ||
| type FL = FieldLength; |
There was a problem hiding this comment.
suggestion(optional): I prefer Symbol, Length over FS, FL for readability.
| second: None, | ||
| time_zone_name: None, | ||
| preferences: None, | ||
| }; |
There was a problem hiding this comment.
suggestion (if-minor): can you implement Default for it, so that one can write:
let bag = Bag {
year: Some(Numeric::Numeric),
month: Some(Month::TwoDigit),
day: Some(Numeric::Numeric),
..Default::default()
};There was a problem hiding this comment.
I agree, and was thinking I need to fix this. There's already an implementation of the Default trait, so I filled a follow-up since it'll touch multiple places across the code.
components/datetime/src/skeleton.rs
Outdated
| /// A [`Skeleton`] is used to match against to find the best pattern. | ||
| /// | ||
| /// This struct implements the [`Copy`] trait, as it's a collection of two references, which should | ||
| /// be fairly cheap to copy. |
There was a problem hiding this comment.
suggestion(optional): The last sentence seems like overdocumentation.
There was a problem hiding this comment.
It turns out the original reason why I was using that trait got factored out when I was working on that. Copy kind of scares me, which is why I documented it like that. I opted for removing both the trait and documentation.
This is a minimal implementation of the UTS 35 skeleton matching algorithm. It will find the best skeleton in the available formats, but it will not modify the final pattern to make it better suit the components::Bag. These modifications I have filed as the follow-ups: #584, #585, #586.
Resolves #481. (Note that this is a scoped down version of that PR, with the follow-ups listed above.)