Add RegionDisplayName that directly implements Writeable#7697
Add RegionDisplayName that directly implements Writeable#7697sffc merged 4 commits intounicode-org:mainfrom
Conversation
|
Is this roughly what you envision for the one-at-a-time display names API? |
|
Discussion: Most/all formatter outputs that implement Writeable borrow from somewhere. With this design, the RegionDisplayName owns its payload. If we add other ways to load RegionDisplayName that borrow instead, we'll need another type. |
sffc
changed the title
| } | ||
| } | ||
|
|
||
| impl writeable::Writeable for RegionDisplayName { |
Manishearth
left a comment
Manishearth
left a comment
There was a problem hiding this comment.
Interesting hybrid.
When I was thinking about designs here I figured we would either have free functions returning strings, or a RegionDisplayName type that returns a borrowed string.
I'm not sure if we get much from this being Writeable. I think it's fine, but a bit roundabout? Perhaps we should add an as_str() API as well, and the Writeable impl is there for consistency
I would rather not, this boxes us in representation-wise. In the future, we might have things like "Korea (North)", "Korea (South)" that are assembled from parts. Also, |
| /// assert_eq!(display_name.to_string(), "United Arab Emirates"); | ||
| /// ``` | ||
| functions: [ | ||
| try_new, |
There was a problem hiding this comment.
please add all constructors
There was a problem hiding this comment.
Not sure which other constructors belong here?
Manishearth
left a comment
Manishearth
left a comment
There was a problem hiding this comment.
I think we should have this in our experimental release.
| /// assert_eq!(display_name.to_string(), "United Arab Emirates"); | ||
| /// ``` | ||
| functions: [ | ||
| try_new, |
There was a problem hiding this comment.
Not sure which other constructors belong here?
Co-authored-by: Gemini CLI <176961590+gemini-code-assist[bot]@users.noreply.github.com>
3 participants
#3260 / #3913
Changelog
icu_experimental/displaynames: Add RegionDisplayName for loading a single display name
RegionDisplayName