ICU-20499 Fixing code path for plural form in MutablePatternModifier.

This commit is contained in:
Shane Carr 2019-03-13 16:08:09 -07:00 committed by Shane F. Carr
parent 56ffae8a0b
commit 2e846616c4
16 changed files with 99 additions and 37 deletions

View file

@ -297,7 +297,7 @@ void CompactHandler::processQuantity(DecimalQuantity &quantity, MicroProps &micr
for (; i < precomputedModsLength; i++) {
const CompactModInfo &info = precomputedMods[i];
if (u_strcmp(patternString, info.patternString) == 0) {
info.mod->applyToMicros(micros, quantity);
info.mod->applyToMicros(micros, quantity, status);
break;
}
}

View file

@ -313,10 +313,8 @@ void LongNameHandler::multiSimpleFormatsToModifiers(const UnicodeString *leadFor
void LongNameHandler::processQuantity(DecimalQuantity &quantity, MicroProps &micros,
UErrorCode &status) const {
parent->processQuantity(quantity, micros, status);
// TODO: Avoid the copy here?
DecimalQuantity copy(quantity);
micros.rounder.apply(copy, status);
micros.modOuter = &fModifiers[utils::getStandardPlural(rules, copy)];
StandardPlural::Form pluralForm = utils::getPluralSafe(micros.rounder, rules, quantity, status);
micros.modOuter = &fModifiers[pluralForm];
}
const Modifier* LongNameHandler::getModifier(int8_t /*signum*/, StandardPlural::Form plural) const {

View file

@ -127,18 +127,16 @@ ImmutablePatternModifier::ImmutablePatternModifier(AdoptingModifierStore* pm, co
void ImmutablePatternModifier::processQuantity(DecimalQuantity& quantity, MicroProps& micros,
UErrorCode& status) const {
parent->processQuantity(quantity, micros, status);
applyToMicros(micros, quantity);
applyToMicros(micros, quantity, status);
}
void ImmutablePatternModifier::applyToMicros(MicroProps& micros, DecimalQuantity& quantity) const {
void ImmutablePatternModifier::applyToMicros(
MicroProps& micros, const DecimalQuantity& quantity, UErrorCode& status) const {
if (rules == nullptr) {
micros.modMiddle = pm->getModifierWithoutPlural(quantity.signum());
} else {
// TODO: Fix this. Avoid the copy.
DecimalQuantity copy(quantity);
copy.roundToInfinity();
StandardPlural::Form plural = utils::getStandardPlural(rules, copy);
micros.modMiddle = pm->getModifier(quantity.signum(), plural);
StandardPlural::Form pluralForm = utils::getPluralSafe(micros.rounder, rules, quantity, status);
micros.modMiddle = pm->getModifier(quantity.signum(), pluralForm);
}
}
@ -164,10 +162,8 @@ void MutablePatternModifier::processQuantity(DecimalQuantity& fq, MicroProps& mi
// This method needs to be const because it overrides a const method in the parent class.
auto nonConstThis = const_cast<MutablePatternModifier*>(this);
if (needsPlurals()) {
// TODO: Fix this. Avoid the copy.
DecimalQuantity copy(fq);
micros.rounder.apply(copy, status);
nonConstThis->setNumberProperties(fq.signum(), utils::getStandardPlural(fRules, copy));
StandardPlural::Form pluralForm = utils::getPluralSafe(micros.rounder, fRules, fq, status);
nonConstThis->setNumberProperties(fq.signum(), pluralForm);
} else {
nonConstThis->setNumberProperties(fq.signum(), StandardPlural::Form::COUNT);
}

View file

@ -46,7 +46,7 @@ class U_I18N_API ImmutablePatternModifier : public MicroPropsGenerator, public U
void processQuantity(DecimalQuantity&, MicroProps& micros, UErrorCode& status) const U_OVERRIDE;
void applyToMicros(MicroProps& micros, DecimalQuantity& quantity) const;
void applyToMicros(MicroProps& micros, const DecimalQuantity& quantity, UErrorCode& status) const;
const Modifier* getModifier(int8_t signum, StandardPlural::Form plural) const;

View file

@ -124,6 +124,23 @@ inline StandardPlural::Form getStandardPlural(const PluralRules *rules,
}
}
/**
* Computes the plural form after copying the number and applying rounding rules.
*/
inline StandardPlural::Form getPluralSafe(
const RoundingImpl& rounder,
const PluralRules* rules,
const DecimalQuantity& dq,
UErrorCode& status) {
// TODO(ICU-20500): Avoid the copy?
DecimalQuantity copy(dq);
rounder.apply(copy, status);
if (U_FAILURE(status)) {
return StandardPlural::Form::OTHER;
}
return getStandardPlural(rules, copy);
}
} // namespace utils
} // namespace impl

View file

@ -90,6 +90,7 @@ class NumberFormatterApiTest : public IntlTestWithFieldPosition {
CurrencyUnit CAD;
CurrencyUnit ESP;
CurrencyUnit PTE;
CurrencyUnit RON;
MeasureUnit METER;
MeasureUnit DAY;

View file

@ -32,6 +32,7 @@ NumberFormatterApiTest::NumberFormatterApiTest(UErrorCode& status)
CAD(u"CAD", status),
ESP(u"ESP", status),
PTE(u"PTE", status),
RON(u"RON", status),
FRENCH_SYMBOLS(Locale::getFrench(), status),
SWISS_SYMBOLS(Locale("de-CH"), status),
MYANMAR_SYMBOLS(Locale("my"), status) {
@ -788,6 +789,14 @@ void NumberFormatterApiTest::unitCurrency() {
Locale("pt-PT"),
444444.55,
u"444,444$55 PTE");
assertFormatSingle(
u"Plural form depending on visible digits (ICU-20499)",
u"currency/RON unit-width-full-name",
NumberFormatter::with().unit(RON).unitWidth(UNUM_UNIT_WIDTH_FULL_NAME),
Locale("ro-RO"),
24,
u"24,00 lei românești");
}
void NumberFormatterApiTest::unitPercent() {

View file

@ -119,7 +119,7 @@ void PatternModifierTest::testPatternWithNoPlaceholder() {
return;
}
DecimalQuantity quantity;
imod->applyToMicros(micros, quantity);
imod->applyToMicros(micros, quantity, status);
micros.modMiddle->apply(nsb, 1, 4, status);
assertSuccess("Spot 7", status);
assertEquals("Safe Path", u"xabcy", nsb.toUnicodeString());
@ -151,7 +151,7 @@ void PatternModifierTest::testMutableEqualsImmutable() {
NumberStringBuilder nsb2;
MicroProps micros2;
LocalPointer<ImmutablePatternModifier> immutable(mod.createImmutable(status));
immutable->applyToMicros(micros2, fq);
immutable->applyToMicros(micros2, fq, status);
micros2.modMiddle->apply(nsb2, 0, 0, status);
assertSuccess("Spot 4", status);

View file

@ -229,6 +229,7 @@ void NumberFormatTest::runIndexedTest( int32_t index, UBool exec, const char* &n
TESTCASE_AUTO(Test20348_CurrencyPrefixOverride);
TESTCASE_AUTO(Test20358_GroupingInPattern);
TESTCASE_AUTO(Test13731_DefaultCurrency);
TESTCASE_AUTO(Test20499_CurrencyVisibleDigitsPlural);
TESTCASE_AUTO_END;
}
@ -9608,4 +9609,17 @@ void NumberFormatTest::Test13731_DefaultCurrency() {
}
}
void NumberFormatTest::Test20499_CurrencyVisibleDigitsPlural() {
IcuTestErrorCode status(*this, "Test20499_CurrencyVisibleDigitsPlural");
LocalPointer<NumberFormat> nf(NumberFormat::createInstance(
"ro-RO", UNumberFormatStyle::UNUM_CURRENCY_PLURAL, status), status);
const char16_t* expected = u"24,00 lei românești";
for (int32_t i=0; i<5; i++) {
UnicodeString actual;
nf->format(24, actual, status);
assertEquals(UnicodeString(u"iteration ") + Int64ToUnicodeString(i),
expected, actual);
}
}
#endif /* #if !UCONFIG_NO_FORMATTING */

View file

@ -295,6 +295,7 @@ class NumberFormatTest: public CalendarTimeZoneTest {
void Test20348_CurrencyPrefixOverride();
void Test20358_GroupingInPattern();
void Test13731_DefaultCurrency();
void Test20499_CurrencyVisibleDigitsPlural();
private:
UBool testFormattableAsUFormattable(const char *file, int line, Formattable &f);

View file

@ -290,10 +290,8 @@ public class LongNameHandler implements MicroPropsGenerator, ModifierStore {
@Override
public MicroProps processQuantity(DecimalQuantity quantity) {
MicroProps micros = parent.processQuantity(quantity);
// TODO: Avoid the copy here?
DecimalQuantity copy = quantity.createCopy();
micros.rounder.apply(copy);
micros.modOuter = modifiers.get(copy.getStandardPlural(rules));
StandardPlural pluralForm = RoundingUtils.getPluralSafe(micros.rounder, rules, quantity);
micros.modOuter = modifiers.get(pluralForm);
return micros;
}

View file

@ -243,11 +243,8 @@ public class MutablePatternModifier implements Modifier, SymbolProvider, MicroPr
if (rules == null) {
micros.modMiddle = pm.getModifierWithoutPlural(quantity.signum());
} else {
// TODO: Fix this. Avoid the copy.
DecimalQuantity copy = quantity.createCopy();
copy.roundToInfinity();
StandardPlural plural = copy.getStandardPlural(rules);
micros.modMiddle = pm.getModifier(quantity.signum(), plural);
StandardPlural pluralForm = RoundingUtils.getPluralSafe(micros.rounder, rules, quantity);
micros.modMiddle = pm.getModifier(quantity.signum(), pluralForm);
}
}
@ -273,10 +270,8 @@ public class MutablePatternModifier implements Modifier, SymbolProvider, MicroPr
public MicroProps processQuantity(DecimalQuantity fq) {
MicroProps micros = parent.processQuantity(fq);
if (needsPlurals()) {
// TODO: Fix this. Avoid the copy.
DecimalQuantity copy = fq.createCopy();
micros.rounder.apply(copy);
setNumberProperties(fq.signum(), copy.getStandardPlural(rules));
StandardPlural pluralForm = RoundingUtils.getPluralSafe(micros.rounder, rules, fq);
setNumberProperties(fq.signum(), pluralForm);
} else {
setNumberProperties(fq.signum(), null);
}

View file

@ -6,7 +6,10 @@ import java.math.BigDecimal;
import java.math.MathContext;
import java.math.RoundingMode;
import com.ibm.icu.impl.StandardPlural;
import com.ibm.icu.number.Precision;
import com.ibm.icu.number.Scale;
import com.ibm.icu.text.PluralRules;
/** @author sffc */
public class RoundingUtils {
@ -219,4 +222,15 @@ public class RoundingUtils {
return null;
}
}
/**
* Computes the plural form after copying the number and applying rounding rules.
*/
public static StandardPlural getPluralSafe(
Precision rounder, PluralRules rules, DecimalQuantity dq) {
// TODO(ICU-20500): Avoid the copy?
DecimalQuantity copy = dq.createCopy();
rounder.apply(copy);
return copy.getStandardPlural(rules);
}
}

View file

@ -128,7 +128,6 @@ public class CompactNotation extends Notation {
magnitude = 0;
micros.rounder.apply(quantity);
} else {
// TODO: Revisit chooseMultiplierAndApply
int multiplier = micros.rounder.chooseMultiplierAndApply(quantity, data);
magnitude = quantity.isZero() ? 0 : quantity.getMagnitude();
magnitude -= multiplier;

View file

@ -4152,7 +4152,7 @@ public class NumberFormatTest extends TestFmwk {
int fracForRoundIncr = 0;
if (roundIncrUsed) {
double testIncr = item.roundIncr;
for (; testIncr > (double)((int)testIncr); testIncr *= 10.0, fracForRoundIncr++);
for (; testIncr > ((int)testIncr); testIncr *= 10.0, fracForRoundIncr++);
}
int minInt = df.getMinimumIntegerDigits();
@ -6614,4 +6614,15 @@ public class NumberFormatTest extends TestFmwk {
assertEquals("currency", "XXX", nf.getCurrency().getCurrencyCode());
}
}
@Test
public void test20499_CurrencyVisibleDigitsPlural() {
ULocale locale = new ULocale("ro-RO");
NumberFormat nf = NumberFormat.getInstance(locale, NumberFormat.PLURALCURRENCYSTYLE);
String expected = "24,00 lei românești";
for (int i=0; i<5; i++) {
String actual = nf.format(24);
assertEquals("iteration " + i, expected, actual);
}
}
}

View file

@ -65,6 +65,7 @@ public class NumberFormatterApiTest {
private static final Currency CAD = Currency.getInstance("CAD");
private static final Currency ESP = Currency.getInstance("ESP");
private static final Currency PTE = Currency.getInstance("PTE");
private static final Currency RON = Currency.getInstance("RON");
@Test
public void notationSimple() {
@ -728,7 +729,7 @@ public class NumberFormatterApiTest {
// NOTE: This is a bit of a hack on CLDR's part. They set the currency symbol to U+200B (zero-
// width space), and they set the decimal separator to the $ symbol.
assertFormatSingle(
"Currency-dependent symbols (Test)",
"Currency-dependent symbols (Test Short)",
"currency/PTE unit-width-short",
NumberFormatter.with().unit(PTE).unitWidth(UnitWidth.SHORT),
ULocale.forLanguageTag("pt-PT"),
@ -736,7 +737,7 @@ public class NumberFormatterApiTest {
"444,444$55 \u200B");
assertFormatSingle(
"Currency-dependent symbols (Test)",
"Currency-dependent symbols (Test Narrow)",
"currency/PTE unit-width-narrow",
NumberFormatter.with().unit(PTE).unitWidth(UnitWidth.NARROW),
ULocale.forLanguageTag("pt-PT"),
@ -744,12 +745,20 @@ public class NumberFormatterApiTest {
"444,444$55 \u200B");
assertFormatSingle(
"Currency-dependent symbols (Test)",
"Currency-dependent symbols (Test ISO Code)",
"currency/PTE unit-width-iso-code",
NumberFormatter.with().unit(PTE).unitWidth(UnitWidth.ISO_CODE),
ULocale.forLanguageTag("pt-PT"),
444444.55,
"444,444$55 PTE");
assertFormatSingle(
"Plural form depending on visible digits (ICU-20499)",
"currency/RON unit-width-full-name",
NumberFormatter.with().unit(RON).unitWidth(UnitWidth.FULL_NAME),
ULocale.forLanguageTag("ro-RO"),
24,
"24,00 lei românești");
}
@Test