diff --git a/icu4c/source/i18n/numrange_impl.cpp b/icu4c/source/i18n/numrange_impl.cpp index 096609bfd41..19b8b10321f 100644 --- a/icu4c/source/i18n/numrange_impl.cpp +++ b/icu4c/source/i18n/numrange_impl.cpp @@ -11,6 +11,7 @@ #include "unicode/numberrangeformatter.h" #include "numrange_impl.h" +#include "patternprops.h" #include "uresimp.h" #include "util.h" @@ -207,6 +208,9 @@ void NumberRangeFormatterImpl::format(UFormattedNumberRangeData& data, bool equa } else { formatterImpl2.preProcess(data.quantity2, micros2, status); } + if (U_FAILURE(status)) { + return; + } // If any of the affixes are different, an identity is not possible // and we must use formatRange(). @@ -392,18 +396,25 @@ void NumberRangeFormatterImpl::formatRange(UFormattedNumberRangeData& data, status); if (U_FAILURE(status)) { return; } lengthInfix = lengthRange - lengthPrefix - lengthSuffix; + U_ASSERT(lengthInfix > 0); // SPACING HEURISTIC // Add spacing unless all modifiers are collapsed. // TODO: add API to control this? + // TODO: Use a data-driven heuristic like currency spacing? + // TODO: Use Unicode [:whitespace:] instead of PatternProps whitespace? (consider speed implications) { bool repeatInner = !collapseInner && micros1.modInner->getCodePointCount() > 0; bool repeatMiddle = !collapseMiddle && micros1.modMiddle->getCodePointCount() > 0; bool repeatOuter = !collapseOuter && micros1.modOuter->getCodePointCount() > 0; if (repeatInner || repeatMiddle || repeatOuter) { - // Add spacing - lengthInfix += string.insertCodePoint(UPRV_INDEX_1, u'\u0020', UNUM_FIELD_COUNT, status); - lengthInfix += string.insertCodePoint(UPRV_INDEX_2, u'\u0020', UNUM_FIELD_COUNT, status); + // Add spacing if there is not already spacing + if (!PatternProps::isWhiteSpace(string.charAt(UPRV_INDEX_1))) { + lengthInfix += string.insertCodePoint(UPRV_INDEX_1, u'\u0020', UNUM_FIELD_COUNT, status); + } + if (!PatternProps::isWhiteSpace(string.charAt(UPRV_INDEX_2 - 1))) { + lengthInfix += string.insertCodePoint(UPRV_INDEX_2, u'\u0020', UNUM_FIELD_COUNT, status); + } } } diff --git a/icu4c/source/test/intltest/numbertest_range.cpp b/icu4c/source/test/intltest/numbertest_range.cpp index dad8a51cee5..1199f886545 100644 --- a/icu4c/source/test/intltest/numbertest_range.cpp +++ b/icu4c/source/test/intltest/numbertest_range.cpp @@ -141,6 +141,54 @@ void NumberRangeFormatterTest::testBasic() { u"~5 000 degrés Fahrenheit", u"5 000–5 000 000 degrés Fahrenheit"); + assertFormatRange( + u"Locale with custom range separator", + NumberRangeFormatter::with(), + Locale("ja"), + u"1～5", + u"~5", + u"~5", + u"0～3", + u"~0", + u"3～3,000", + u"3,000～5,000", + u"4,999～5,001", + u"~5,000", + u"5,000～5,000,000"); + + assertFormatRange( + u"Locale that already has spaces around range separator", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_NONE) + .numberFormatterBoth(NumberFormatter::with().unit(KELVIN)), + Locale("hr"), + u"1 K – 5 K", + u"~5 K", + u"~5 K", + u"0 K – 3 K", + u"~0 K", + u"3 K – 3.000 K", + u"3.000 K – 5.000 K", + u"4.999 K – 5.001 K", + u"~5.000 K", + u"5.000 K – 5.000.000 K"); + + assertFormatRange( + u"Locale with custom numbering system and no plural ranges data", + NumberRangeFormatter::with(), + Locale("shn@numbers=beng"), + // 012459 = ০১৩৪৫৯ + u"১–৫", + u"~৫", + u"~৫", + u"০–৩", + u"~০", + u"৩–৩,০০০", + u"৩,০০০–৫,০০০", + u"৪,৯৯৯–৫,০০১", + u"~৫,০০০", + u"৫,০০০–৫,০০০,০০০"); + assertFormatRange( u"Portuguese currency", NumberRangeFormatter::with() diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java index c05a9155711..71aafe51cd1 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java @@ -4,6 +4,7 @@ package com.ibm.icu.number; import com.ibm.icu.impl.ICUData; import com.ibm.icu.impl.ICUResourceBundle; +import com.ibm.icu.impl.PatternProps; import com.ibm.icu.impl.SimpleFormatterImpl; import com.ibm.icu.impl.StandardPlural; import com.ibm.icu.impl.UResource; @@ -287,18 +288,25 @@ class NumberRangeFormatterImpl { PrefixInfixSuffixLengthHelper h = new PrefixInfixSuffixLengthHelper(); SimpleModifier.formatTwoArgPattern(fRangePattern, string, 0, h, null); + assert h.lengthInfix > 0; // SPACING HEURISTIC // Add spacing unless all modifiers are collapsed. // TODO: add API to control this? + // TODO: Use a data-driven heuristic like currency spacing? + // TODO: Use Unicode [:whitespace:] instead of PatternProps whitespace? (consider speed implications) { boolean repeatInner = !collapseInner && micros1.modInner.getCodePointCount() > 0; boolean repeatMiddle = !collapseMiddle && micros1.modMiddle.getCodePointCount() > 0; boolean repeatOuter = !collapseOuter && micros1.modOuter.getCodePointCount() > 0; if (repeatInner || repeatMiddle || repeatOuter) { - // Add spacing - h.lengthInfix += string.insertCodePoint(h.index1(), '\u0020', null); - h.lengthInfix += string.insertCodePoint(h.index2(), '\u0020', null); + // Add spacing if there is not already spacing + if (!PatternProps.isWhiteSpace(string.charAt(h.index1()))) { + h.lengthInfix += string.insertCodePoint(h.index1(), '\u0020', null); + } + if (!PatternProps.isWhiteSpace(string.charAt(h.index2() - 1))) { + h.lengthInfix += string.insertCodePoint(h.index2(), '\u0020', null); + } } } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberRangeFormatterTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberRangeFormatterTest.java index 21e599d71b0..52933d0b271 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberRangeFormatterTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberRangeFormatterTest.java @@ -129,6 +129,54 @@ public class NumberRangeFormatterTest { "~5 000 degrés Fahrenheit", "5 000–5 000 000 degrés Fahrenheit"); + assertFormatRange( + "Locale with custom range separator", + NumberRangeFormatter.with(), + new ULocale("ja"), + "1～5", + "~5", + "~5", + "0～3", + "~0", + "3～3,000", + "3,000～5,000", + "4,999～5,001", + "~5,000", + "5,000～5,000,000"); + + assertFormatRange( + "Locale that already has spaces around range separator", + NumberRangeFormatter.with() + .collapse(RangeCollapse.NONE) + .numberFormatterBoth(NumberFormatter.with().unit(MeasureUnit.KELVIN)), + new ULocale("hr"), + "1 K – 5 K", + "~5 K", + "~5 K", + "0 K – 3 K", + "~0 K", + "3 K – 3.000 K", + "3.000 K – 5.000 K", + "4.999 K – 5.001 K", + "~5.000 K", + "5.000 K – 5.000.000 K"); + + assertFormatRange( + "Locale with custom numbering system and no plural ranges data", + NumberRangeFormatter.with(), + new ULocale("shn@numbers=beng"), + // 012459 = ০১৩৪৫৯ + "১–৫", + "~৫", + "~৫", + "০–৩", + "~০", + "৩–৩,০০০", + "৩,০০০–৫,০০০", + "৪,৯৯৯–৫,০০১", + "~৫,০০০", + "৫,০০০–৫,০০০,০০০"); + assertFormatRange( "Portuguese currency", NumberRangeFormatter.with()