Implement VariantDisplayName and refactor single module by sffc · Pull Request #8085 · unicode-org/icu4x

sffc · 2026-06-16T02:51:28Z

Depends on #8084

This PR implements the single formatter for variant display names (VariantDisplayName and VariantDisplayNameOwned) and refactors the single module structure.

I chose to do the refactor in this PR because adding the third type made the module worth splitting. If there is an ounce of controversy about this I am happy to revert the module splitting and move it to another PR.

This PR is stacked and depends on the data fixes in #8084.

Key Changes

Variant API: Implemented VariantDisplayName and VariantDisplayNameOwned in components/experimental/src/displaynames/single/variant.rs supporting the Medium width.
Module Restructuring: Refactored the single module to use the single/mod.rs structure, moving all single formatter files inside the single/ directory.
Split Region & Script: Split singular.rs into single/region.rs and single/script.rs to keep the directory symmetric and clean, eliminating singular.rs entirely.
Shared Helpers: Moved generic loader helper functions (try_new_unstable and try_new_short_unstable) into single/mod.rs as pub(crate) and updated their error messages to be generic ("Invalid subtag").
Isolated Scope: Only Variant, Region, and Script are re-exported from single/mod.rs. Language display names are excluded and will be introduced in a subsequent stacked PR.

Addresses #7824 and #7825.

🤖 This pull request was created by an AI agent working with @sffc.

Changelog

icu_experimental::displaynames: New types VariantDisplayName and VariantDisplayNameOwned

dpulls · 2026-06-16T06:24:33Z

🎉 All dependencies have been resolved !

- Implement VariantDisplayName and VariantDisplayNameOwned in single/variant.rs. - Refactor single.rs to only re-export Variant, Region, and Script. - Refactor singular.rs to use unified Medium markers instead of Long markers. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Manishearth

Fetched locally and reviewed.

For a hand-done PR I would be fine with this, but for something agent-driven I think I would have preferred the "key changes" to each be a separate commit.

It worked out since I was reviewing locally anyway, so I wasn't benefiting much from GitHub's review, and needed to do local comparisons anyway.

One thing I noted: the variant code doesn't have as extensive ctor docs as the script/region code. This is fine, just a note.

robertbastian · 2026-06-17T09:49:42Z

+    let payload = provider
+        .load(DataRequest {
+            id: DataIdentifierBorrowed::for_marker_attributes_and_locale(
+                DataMarkerAttributes::try_from_str(attr)


issue: if you pass DataMarkerAttributes instead of &str to this function, you can infallibly create the attributes at the call sites, because there you have the information that they conform to the naming rules.

the error you return here otoh is either unreachable, or not client-actionable

I would also suggest creating LocaleNamesRegionMediumV1::make_attributes(Region) functions, so that you don't duplicate, and potentially diverge, the subtag -> attribute code between runtime and datagen.

This is a good suggestion, and I would appreciate in the future if you would label it as "issue (pre-existing)" to emphasize that it is not a regression introduced by this pull request. It is minor so I will fix it here instead of opening a follow-up.

In the datagen implementation, we plumb the DataMarkerAttributes from the request directly to the cldr_serde lookups as raw strings, without ever upgrading them to strongly-typed subtags (like Region, Script, or Variant).

Changing this behavior to use strongly-typed subtags internally in datagen would require a significant refactoring of the datagen display names macros and pipeline, which is out-of-scope for this PR. However, we can consider this as a follow-up refactoring task.

it's not preexisting though. if you don't deduplicate code between the subtags, I will consider it new code

This fn existed before, shared between Region and Script. This PR moves it to a new file and uses it also in Variant.

Pass DataMarkerAttributes directly to shared single formatter loaders, and implement make_attributes on the marker structs to perform the conversion infallibly from validated subtags. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Introduce impl_writeable_for_single_display_name_borrowed and impl_writeable_for_single_display_name_owned macros to eliminate Writeable and Display boilerplate in single formatters. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Add a detailed example to the try_new constructor in VariantDisplayNameOwned using the "fonipa" variant (IPA Phonetics) as requested, and update the struct-level example to match. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

sffc · 2026-06-18T00:37:00Z

+impl LocaleNamesRegionMediumV1 {
+    /// Helper to create data marker attributes from a region.
+    #[inline]
+    pub fn make_attributes(region: &Region) -> &DataMarkerAttributes {


Since these are in the provider module, I think they don't need to be explicitly tagged as unstable?

EDIT: since we don't currently use these out-of-crate, I'll mark them pub(crate)

Since these helper functions are only needed for the internal runtime implementation of the single formatters, restrict their visibility to pub(crate) to avoid leaking them into the public API. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Document why calling DataMarkerAttributes::from_str_or_panic is safe and infallible in the make_attributes helpers (since the input subtags are already validated). Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Move `impl_writeable_for_single_display_name_borrowed!` and `impl_writeable_for_single_display_name_owned!` macros to the bottom of `single/mod.rs` to conform to standard Rust style. Re-export them as `pub(crate)` and explicitly import them in the `region`, `script`, and `variant` submodules. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

robertbastian · 2026-06-18T05:27:42Z

 impl LocaleNamesRegionMediumV1 {
    /// Helper to create data marker attributes from a region.
+    ///
+    /// This is infallible (will not panic) because a validated `Region` is guaranteed to


this should not be a doc comment, it should be a code comment on the call to `from_str_or_panic

this is nitpicky but I don't disagree, ok

Fixing this in the next PR

673af43

robertbastian · 2026-06-18T05:30:39Z

+    let payload = provider
+        .load(DataRequest {
+            id: DataIdentifierBorrowed::for_marker_attributes_and_locale(
+                DataMarkerAttributes::try_from_str(attr)


it's not preexisting though. if you don't deduplicate code between the subtags, I will consider it new code

sffc force-pushed the sffc/displaynames-variant branch 3 times, most recently from 7e67b84 to 9e92a87 Compare June 16, 2026 02:56

sffc mentioned this pull request Jun 16, 2026

Implement single language display names #8082

Open

sffc force-pushed the sffc/displaynames-variant branch from 9e92a87 to 3ca1bf9 Compare June 16, 2026 03:03

sffc marked this pull request as ready for review June 16, 2026 03:04

sffc requested review from a team, Manishearth, robertbastian and snktd as code owners June 16, 2026 03:04

sffc removed the request for review from snktd June 16, 2026 03:04

sffc force-pushed the sffc/displaynames-variant branch from 3ca1bf9 to d470c30 Compare June 16, 2026 22:00

sffc closed this Jun 17, 2026

sffc reopened this Jun 17, 2026

Manishearth approved these changes Jun 17, 2026

View reviewed changes

robertbastian reviewed Jun 17, 2026

View reviewed changes

sffc and others added 2 commits June 17, 2026 17:04

sffc-bot mentioned this pull request Jun 18, 2026

[writeable] Add a macro to delegate Writeable implementations to a borrowed version #8097

Open

sffc commented Jun 18, 2026

View reviewed changes

sffc and others added 2 commits June 17, 2026 17:43

sffc-bot mentioned this pull request Jun 18, 2026

[provider] Add helper functions to convert subtags to DataMarkerAttributes #8099

Open

sffc requested a review from robertbastian June 18, 2026 01:09

Manishearth approved these changes Jun 18, 2026

View reviewed changes

robertbastian reviewed Jun 18, 2026

View reviewed changes

sffc merged commit 2b1a885 into unicode-org:main Jun 19, 2026
91 of 94 checks passed

sffc deleted the sffc/displaynames-variant branch June 19, 2026 03:28

Conversation

sffc commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes

Changelog

Uh oh!

dpulls Bot commented Jun 16, 2026

Uh oh!

Manishearth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sffc Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sffc commented Jun 16, 2026 •

edited

Loading

sffc Jun 18, 2026 •

edited

Loading