Skip to content

Add APIs and tests for currency sign options to CurrencyFormatter#7790

Open
younies wants to merge 1 commit intounicode-org:mainfrom
younies:currecy-sign-options
Open

Add APIs and tests for currency sign options to CurrencyFormatter#7790
younies wants to merge 1 commit intounicode-org:mainfrom
younies:currecy-sign-options

Conversation

@younies
Copy link
Copy Markdown
Member

@younies younies commented Mar 18, 2026

Summary

  • Introduced CurrencySign enum to control formatting of negative currency values, supporting both standard and accounting styles.
  • Updated CurrencyFormatterOptions to include a currency_sign field, defaulting to Standard.
  • Added tests for accounting style formatting in test_en_us_accounting to implement the new behaviour in the next PR

Changelog

icu_experimental/currency: Add options for accounting currency sign

  • New enum: CurrencySign
  • New field CurrencyFormatterOptions::currency_sign
  • New impl: impl From<CurrencySign> for CurrencyFormatterOptions

- Introduced `CurrencySign` enum to control formatting of negative currency values, supporting both standard and accounting styles.
- Updated `CurrencyFormatterOptions` to include a `currency_sign` field, defaulting to `Standard`.
- Added tests for accounting style formatting in `test_en_us_accounting`.
@younies younies requested a review from sffc March 18, 2026 16:02

/// The sign style for negative currency values.
/// Default is [`CurrencySign::Standard`].
pub currency_sign: CurrencySign,
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.

Nit: All options should be optional

Suggested change
pub currency_sign: CurrencySign,
pub currency_sign: Option<CurrencySign>,

Probably width should be optional, too.

@sffc sffc changed the title feat: Add currency sign options to CurrencyFormatter Add APIs and tests for currency sign options to CurrencyFormatter Mar 19, 2026
@sffc
Copy link
Copy Markdown
Member

sffc commented Mar 19, 2026

Note: the changelog needs to follow a specific format. You had put:

  • Introduced CurrencySign enum to support standard and accounting currency sign styles.
  • Added currency_sign to CurrencyFormatterOptions, with Standard as the default.
  • Added en-US accounting-style formatting tests.

I changed it, according to https://github.com/unicode-org/icu4x/blob/main/documents/process/changelog.md

Also, now that we have the ## Changelog section with the specific APIs, your ## Summary section (which doesn't need a header) should describe what the PR accomplishes at a high level.

@robertbastian robertbastian self-requested a review April 7, 2026 11:14
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