Change content_locale to be &LanguageIdentifier #5566
Conversation
There was a problem hiding this comment.
Not a fan of lifetimes on options bags but I do think this is better.
I could be wrong but I recall us having a long discussion about this years ago and coming to a conclusion. I believe the conclusion was to borrow owned types in options bags but worth seeing if we can find and document it somewhere.
|
Yep, I don't see any strong justification for this to not be |
|
Looks like dart bindings generation has broken? Dart update? cc @robertbastian I support bypassing test-dart and merging this (and other open PRs that don't touch ffi) |
|
@aethanyc @makotokato Any feedback? |
| @@ -2,8 +2,10 @@ | |||
| // called LICENSE at the top level of the ICU4X source tree | |||
| // (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ). | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Nit: this blank line is not needed.
| use icu_locale_core::langid; | ||
| use icu_properties::PropertyNames; | ||
| use icu_properties::PropertyNamesLong; | ||
|
|
There was a problem hiding this comment.
Nit: this blank line is not needed.
|
Currently, segmenter doesn't require country and etc, so it is OK. |
Region is still part of the LanguageIdentifier. |
This keeps the options bags
Copy.I also switched to
LanguageIdentifierwhich is a more public-facing, stable type. We're trying to phase outDataLocalein public-facing APIs (#419).