Skip to content

Commit 757be35

Browse files
sffcsffc
committed
ICU-23110 Fix number range formatting with percentages
See #3483
1 parent 08a0746 commit 757be35

File tree

7 files changed

+61
-14
lines changed

7 files changed

+61
-14
lines changed

‎icu4c/source/i18n/numrange_impl.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,8 @@ void NumberRangeFormatterImpl::format(UFormattedNumberRangeData& data, bool equa
160160
return;
161161
}
162162

163+
DecimalQuantity quantityBackup(data.quantity1);
164+
163165
MicroProps micros1;
164166
MicroProps micros2;
165167
formatterImpl1.preProcess(data.quantity1, micros1, status);
@@ -216,7 +218,7 @@ void NumberRangeFormatterImpl::format(UFormattedNumberRangeData& data, bool equa
216218
UNUM_IDENTITY_RESULT_EQUAL_BEFORE_ROUNDING):
217219
case identity2d(UNUM_IDENTITY_FALLBACK_APPROXIMATELY_OR_SINGLE_VALUE,
218220
UNUM_IDENTITY_RESULT_EQUAL_AFTER_ROUNDING):
219-
formatApproximately(data, micros1, micros2, status);
221+
formatApproximately(data, quantityBackup, micros1, micros2, status);
220222
break;
221223

222224
case identity2d(UNUM_IDENTITY_FALLBACK_APPROXIMATELY_OR_SINGLE_VALUE,
@@ -248,15 +250,15 @@ void NumberRangeFormatterImpl::formatSingleValue(UFormattedNumberRangeData& data
248250

249251

250252
void NumberRangeFormatterImpl::formatApproximately (UFormattedNumberRangeData& data,
253+
DecimalQuantity quantity,
251254
MicroProps& micros1, MicroProps& micros2,
252255
UErrorCode& status) const {
253256
if (U_FAILURE(status)) { return; }
254257
if (fSameFormatters) {
255258
// Re-format using the approximately formatter:
256259
MicroProps microsAppx;
257-
data.quantity1.resetExponent();
258-
fApproximatelyFormatter.preProcess(data.quantity1, microsAppx, status);
259-
int32_t length = NumberFormatterImpl::writeNumber(microsAppx.simple, data.quantity1, data.getStringRef(), 0, status);
260+
fApproximatelyFormatter.preProcess(quantity, microsAppx, status);
261+
int32_t length = NumberFormatterImpl::writeNumber(microsAppx.simple, quantity, data.getStringRef(), 0, status);
260262
length += microsAppx.modInner->apply(data.getStringRef(), 0, length, status);
261263
length += microsAppx.modMiddle->apply(data.getStringRef(), 0, length, status);
262264
microsAppx.modOuter->apply(data.getStringRef(), 0, length, status);

‎icu4c/source/i18n/numrange_impl.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ class NumberRangeFormatterImpl : public UMemory {
6464
UErrorCode& status) const;
6565

6666
void formatApproximately(UFormattedNumberRangeData& data,
67+
DecimalQuantity quantity,
6768
MicroProps& micros1, MicroProps& micros2,
6869
UErrorCode& status) const;
6970

‎icu4c/source/test/intltest/numbertest.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ class NumberRangeFormatterTest : public IntlTestWithFieldPosition {
334334
void test21683_StateLeak();
335335
void testCreateLNRFFromNumberingSystemInSkeleton();
336336
void test22288_DifferentStartEndSettings();
337+
void test23110_PercentApproximately();
337338

338339
void runIndexedTest(int32_t index, UBool exec, const char*& name, char* par = nullptr) override;
339340

‎icu4c/source/test/intltest/numbertest_range.cpp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ void NumberRangeFormatterTest::runIndexedTest(int32_t index, UBool exec, const c
6060
TESTCASE_AUTO(test21683_StateLeak);
6161
TESTCASE_AUTO(testCreateLNRFFromNumberingSystemInSkeleton);
6262
TESTCASE_AUTO(test22288_DifferentStartEndSettings);
63+
TESTCASE_AUTO(test23110_PercentApproximately);
6364
TESTCASE_AUTO_END;
6465
}
6566

@@ -1180,6 +1181,26 @@ void NumberRangeFormatterTest::test22288_DifferentStartEndSettings() {
11801181
assertEquals("Should format successfully", u"2–3 US dollars", result.toString(status));
11811182
}
11821183

1184+
void NumberRangeFormatterTest::test23110_PercentApproximately() {
1185+
IcuTestErrorCode status(*this, "test23110_PercentApproximately");
1186+
1187+
assertFormatRange(
1188+
u"Approximately percentage formatting",
1189+
NumberRangeFormatter::with()
1190+
.numberFormatterBoth(NumberFormatter::forSkeleton(u"%x100", status)),
1191+
Locale("en-US"),
1192+
u"100% – 500%",
1193+
u"499.99999% – 500.00001%",
1194+
u"~500%", // was returning "~50,000%"
1195+
u"0% – 300%",
1196+
u"~0%",
1197+
u"300% – 300,000%",
1198+
u"300,000% – 500,000%",
1199+
u"499,900% – 500,100%",
1200+
u"~500,000%",
1201+
u"500,000% – 500,000,000%");
1202+
}
1203+
11831204
void NumberRangeFormatterTest::assertFormatRange(
11841205
const char16_t* message,
11851206
const UnlocalizedNumberRangeFormatter& f,

‎icu4j/main/common_tests/src/test/java/com/ibm/icu/dev/test/number/NumberRangeFormatterTest.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,6 +1050,25 @@ public void test22288_DifferentStartEndSettings() {
10501050
assertEquals("Should format successfully", "2–3 US dollars", result.toString());
10511051
}
10521052

1053+
@Test
1054+
public void Test23110_PercentApproximately() {
1055+
assertFormatRange(
1056+
"Approximately percentage formatting",
1057+
NumberRangeFormatter.with()
1058+
.numberFormatterBoth(NumberFormatter.forSkeleton("%x100")),
1059+
ULocale.forLanguageTag("en-US"),
1060+
"100% – 500%",
1061+
"499.99999% – 500.00001%",
1062+
"~500%", // was returning "~50,000%"
1063+
"0% – 300%",
1064+
"~0%",
1065+
"300% – 300,000%",
1066+
"300,000% – 500,000%",
1067+
"499,900% – 500,100%",
1068+
"~500,000%",
1069+
"500,000% – 500,000,000%");
1070+
}
1071+
10531072
static void assertFormatRange(
10541073
String message,
10551074
UnlocalizedNumberRangeFormatter f,

‎icu4j/main/core/src/main/java/com/ibm/icu/number/NumberFormatterSettings.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -655,12 +655,14 @@ MacroProps resolve() {
655655
long seen = 0;
656656
NumberFormatterSettings<?> current = this;
657657
while (current != null) {
658-
long keyBitmask = (1L << current.key);
659-
if (0 != (seen & keyBitmask)) {
660-
current = current.parent;
661-
continue;
658+
if (current.key != KEY_MACROS) {
659+
long keyBitmask = (1L << current.key);
660+
if (0 != (seen & keyBitmask)) {
661+
current = current.parent;
662+
continue;
663+
}
664+
seen |= keyBitmask;
662665
}
663-
seen |= keyBitmask;
664666
switch (current.key) {
665667
case KEY_MACROS:
666668
macros.fallback((MacroProps) current.value);

‎icu4j/main/core/src/main/java/com/ibm/icu/number/NumberRangeFormatterImpl.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ public NumberRangeFormatterImpl(RangeMacroProps macros) {
178178
}
179179

180180
public FormattedNumberRange format(DecimalQuantity quantity1, DecimalQuantity quantity2, boolean equalBeforeRounding) {
181+
DecimalQuantity quantityBackup = quantity1.createCopy();
182+
181183
FormattedStringBuilder string = new FormattedStringBuilder();
182184
MicroProps micros1 = formatterImpl1.preProcess(quantity1);
183185
MicroProps micros2;
@@ -224,7 +226,7 @@ public FormattedNumberRange format(DecimalQuantity quantity1, DecimalQuantity qu
224226
case (2 | (1 << 4)): // APPROXIMATELY, EQUAL_AFTER_ROUNDING
225227
case (2 | (0 << 4)): // APPROXIMATELY, EQUAL_BEFORE_ROUNDING
226228
case (1 | (1 << 4)): // APPROXIMATE_OR_SINGLE_VALUE, EQUAL_AFTER_ROUNDING
227-
formatApproximately(quantity1, quantity2, string, micros1, micros2);
229+
formatApproximately(quantityBackup, quantity1, quantity2, string, micros1, micros2);
228230
break;
229231

230232
case (1 | (0 << 4)): // APPROXIMATE_OR_SINGLE_VALUE, EQUAL_BEFORE_ROUNDING
@@ -252,13 +254,12 @@ private void formatSingleValue(DecimalQuantity quantity1, DecimalQuantity quanti
252254

253255
}
254256

255-
private void formatApproximately(DecimalQuantity quantity1, DecimalQuantity quantity2, FormattedStringBuilder string,
257+
private void formatApproximately(DecimalQuantity quantityBackup, DecimalQuantity quantity1, DecimalQuantity quantity2, FormattedStringBuilder string,
256258
MicroProps micros1, MicroProps micros2) {
257259
if (fSameFormatters) {
258260
// Re-format using the approximately formatter:
259-
quantity1.resetExponent();
260-
MicroProps microsAppx = fApproximatelyFormatter.preProcess(quantity1);
261-
int length = NumberFormatterImpl.writeNumber(microsAppx, quantity1, string, 0);
261+
MicroProps microsAppx = fApproximatelyFormatter.preProcess(quantityBackup);
262+
int length = NumberFormatterImpl.writeNumber(microsAppx, quantityBackup, string, 0);
262263
// HEURISTIC: Desired modifier order: inner, middle, approximately, outer.
263264
length += microsAppx.modInner.apply(string, 0, length);
264265
length += microsAppx.modMiddle.apply(string, 0, length);

0 commit comments

Comments
 (0)