Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename FixedDecimalFormatter to DecimalFormatter #5953

Open
robertbastian opened this issue Jan 6, 2025 · 2 comments
Open

Rename FixedDecimalFormatter to DecimalFormatter #5953

robertbastian opened this issue Jan 6, 2025 · 2 comments
Labels
2.0-breaking Changes that are breaking API changes discuss Discuss at a future ICU4X-SC meeting

Comments

@robertbastian
Copy link
Member

It now accepts SignedFixedDecimal, but SignedFixedDecimalFormatter is a mouthful. Other components have names that are more aligned with the component names than the inputs as well.

@robertbastian robertbastian added discuss Discuss at a future ICU4X-SC meeting 2.0-breaking Changes that are breaking API changes labels Jan 6, 2025
@sffc
Copy link
Member

sffc commented Jan 9, 2025

I named it FixedDecimalFormatter in case we wanted to reserve DecimalFormatter for the kitchen sink (#275).

@Manishearth
Copy link
Member

Start 11:12

  • @sffc We can't really discuss the renaming until we decide how many formatters to have. We'll have at least 6-7 composite decimal types. Do we have separate formatters for them? Do they have different payloads?
  • @younies I think we should have fewer formatters. DurationFormatter needs both of them.
  • @Manishearth The benefit of splitting the data does not really justify the cost of having more types. At most we should have multiple constructors.

Tentative conclusion:

  • One formatter type for WithNan<WithInfinity<SignedFixedDecimal>>
  • Probably keep scientific in a different type
  • Maybe have a new payload inside the formatter based on implementation details, and optimize it in various ways
  • Do not have multiple constructors that put the formatter in funny states

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0-breaking Changes that are breaking API changes discuss Discuss at a future ICU4X-SC meeting
Projects
None yet
Development

No branches or pull requests

3 participants