Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before
Problem
Cldr.Number.Symbolwithstruct/2, which drops every CLDR symbol the struct has no field for. Nobody sees what it drops.approximately_sign. CLDR says to format an approximate number with this sign where the minus sign goes. The "approximately" pattern that we use instead is "not currently in use" (UTS ** (KeyError) key :symbol_count not found in: nil. #35), and in 60 locales it has other text (fr-BE sign "≃", pattern "≈{0}").:currency_long_with_symbolformats the number twice, so a negative number gets two minus signs.After
nb now gives "ca.1 234" with no space, the same as ICU.
Details
Cldr.Number.Symbol.new!/1leaves outtime_separator_alt_variantby name (the "." of nb, nn, no and sv; no formatter reads it). It also leaves outcurrency_decimalandcurrency_group, but only while the struct has no field for them. Use CLDR currency separators when formatting a currency #66 adds those fields, so the two PRs can merge in either order. Any other unknown symbol raises when the backend compiles. All 840 symbol maps in the 766 locales build.Cldr.Config.number_symbol!/1to ex_cldr with the same rules. Once ex_cldr releases it, the backend callsCldr.Config.number_symbols_for!/2andnew!/1goes (markedTODO(revert)).PatternStringUtils::patternInfoToStringBuilder). A positive number uses the negative pattern with the sign in place of the minus sign. A negative number gets the sign before the minus sign. When the negative pattern has no minus sign, ICU and this change put the sign before the positive pattern for a positive number and add no sign for a negative number: "~$1,234.00" and "($1,234.00)" in accounting. Spellout, roman and other RBNF formats have no minus position, so they keep the "approximately" pattern.:currency_long_with_symbolnow formats the inner long number unsigned and without the approximately flag. The outer currency pattern adds the sign once. Take the sign from each number when formatting with validated options #65 changes how the sign pattern is picked. This fix does not depend on it.to_string/3: "0". ICU gives "-0".test/number/approximately_test.exspins ICU 78.3 strings (NodeIntl.NumberFormat#formatRange(n, n)) for fr-BE, nb, en zero, long currency and accounting. It also covers nb spellout and roman, Decimal -0, and the one sign in:currency_long_with_symbol.test/number/number_symbol_test.exschecks that nb keeps "ca." and loses the "." time separator, and that an unknown symbol raises.mix test: 5244 passed (282 doctests, 4962 tests).mix compile --forceshows no warnings. The changed lines are formatted.mix format --check-formattedalso fails on upstreammain, on lines this change does not touch.AI disclosure: AI models helped us find this issue, write the change, and review it.