Skip to content

feat(currency): Merge all currency formatters into unified CurrencyFormatter<V> using AbstractFormatter#8189

Merged
robertbastian merged 3 commits into
unicode-org:mainfrom
robertbastian:ccf
Jul 8, 2026
Merged

feat(currency): Merge all currency formatters into unified CurrencyFormatter<V> using AbstractFormatter#8189
robertbastian merged 3 commits into
unicode-org:mainfrom
robertbastian:ccf

Conversation

@robertbastian

@robertbastian robertbastian commented Jul 7, 2026

Copy link
Copy Markdown
Member

Summary

This PR merges all remaining currency formatters into a single unified CurrencyFormatter<V> type by leveraging AbstractFormatter, proposing an alternative architecture to #8182.
Instead of maintaining separate representation structs or duplicating formatting data (such as decimal_formatter, compact_data, and plural_rules), CurrencyFormatter<V> directly wraps an underlying value formatter (DecimalFormatter for standard currency formatting or CompactDecimalFormatter for compact currency formatting). This unifies the currency formatting architecture, simplifies pattern interpolation, and significantly reduces boilerplate across the module.

Changelog

  • Architecture Unification: Refactored CurrencyFormatter<V> to constrain V: AbstractFormatter, allowing it to wrap either DecimalFormatter or CompactDecimalFormatter directly in a single value_formatter: V field.
  • Constructor Streamlining:
    • Standard constructors (try_new_short, try_new_narrow, try_new_long) initialize CurrencyFormatter with DecimalFormatter.
    • Compact constructors (try_new_compact_short, try_new_compact_narrow, try_new_compact_long) initialize CurrencyFormatter with CompactDecimalFormatter.
  • icu_decimal Delegation: Exposed pub(crate) helper methods (format_unsigned, format_sign) and exported FormattedUnsignedCompactDecimal / AbstractFormatter from icu_decimal so currency formatters can delegate numeric formatting directly to the underlying value formatter.
  • Boilerplate & Code Reduction: Removed redundant data fields and simplified formatting logic across compact_formatter.rs and formatter.rs, resulting in a net code reduction of over 200 lines (+665 additions, -876 deletions).

@robertbastian robertbastian force-pushed the ccf branch 2 times, most recently from a49a0ae to 5414c4b Compare July 8, 2026 10:11
@robertbastian robertbastian marked this pull request as ready for review July 8, 2026 10:44
@younies younies changed the title Merge all remaining currency formatters into one type feat(currency): Merge all currency formatters into unified CurrencyFormatter<V> using AbstractFormatter Jul 8, 2026

@younies younies left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @robertbastian for putting this together! This architectural direction aligns really well with my thinking—specifically, making the decimal formatter act as the underlying foundational engine that powers both currency formatters and, eventually, unit formatters. This approach will be especially beneficial once the compact currency pattern gluing proposal (CLDR-19617) is accepted.

I'm going to update the NumberFormatter design document accordingly so we can discuss and align on this new approach in our next meeting.

A few housekeeping updates:

  • I fixed a minor clippy issue (an unused import), verified that formatting and all workspace tests pass cleanly, and pushed the fix to the branch.
  • I updated the PR title and description to follow our conventional commits format and better reflect the architectural changes—feel free to revise them!
  • I left one inline review comment for your thoughts.

FormattedUnsignedDecimal,
};

pub trait Sealed {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add documentation

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub trait Sealed {}
/// An un-exportable supertrait used to seal [`AbstractFormatter`].
///
/// Because this trait is not re-exported from the crate root, downstream crates cannot
/// implement it. This prevents third-party implementations of [`AbstractFormatter`],
/// preserving SemVer stability and allowing internal formatting methods to evolve
/// without breaking external code.
pub trait Sealed {}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will follow-up

TAG=agy
CONV=85455991-6f0d-44c3-87be-4411f043a296
@robertbastian robertbastian enabled auto-merge (squash) July 8, 2026 13:47
@robertbastian robertbastian disabled auto-merge July 8, 2026 13:54
@robertbastian robertbastian merged commit ef14854 into unicode-org:main Jul 8, 2026
34 checks passed
@robertbastian robertbastian deleted the ccf branch July 8, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants