From b62200061c217689bab56482d71334b4e81ab0bf Mon Sep 17 00:00:00 2001 From: Victor Chang Date: Fri, 14 Sep 2018 14:24:05 +0100 Subject: [PATCH] ICU-20058 Fix mimimum significant digits in engineering notation - Follow the spec to calculate the mimimum significant digits in engineering notation - The bug is regression since ICU 58. The new test still passes on ICU58-based DecimalFormat - Maximum significant digits is not changed --- icu4c/source/i18n/number_mapper.cpp | 16 +++++++++++----- .../testdata/numberformattestspecification.txt | 9 +++++++++ .../com/ibm/icu/number/NumberPropertyMapper.java | 15 +++++++++++---- .../dev/data/numberformattestspecification.txt | 9 +++++++++ 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/icu4c/source/i18n/number_mapper.cpp b/icu4c/source/i18n/number_mapper.cpp index d260632f93b..2c9a8e5178f 100644 --- a/icu4c/source/i18n/number_mapper.cpp +++ b/icu4c/source/i18n/number_mapper.cpp @@ -225,8 +225,8 @@ MacroProps NumberPropertyMapper::oldToNew(const DecimalFormatProperties& propert // TODO: Overriding here is a bit of a hack. Should this logic go earlier? if (macros.precision.fType == Precision::PrecisionType::RND_FRACTION) { // For the purposes of rounding, get the original min/max int/frac, since the local - // variables - // have been manipulated for display purposes. + // variables have been manipulated for display purposes. + int maxInt_ = properties.maximumIntegerDigits; int minInt_ = properties.minimumIntegerDigits; int minFrac_ = properties.minimumFractionDigits; int maxFrac_ = properties.maximumFractionDigits; @@ -237,9 +237,15 @@ MacroProps NumberPropertyMapper::oldToNew(const DecimalFormatProperties& propert // Patterns like "#.##E0" (no zeros in the mantissa), which mean round to maxFrac+1 macros.precision = Precision::constructSignificant(1, maxFrac_ + 1).withMode(roundingMode); } else { - // All other scientific patterns, which mean round to minInt+maxFrac - macros.precision = Precision::constructSignificant( - minInt_ + minFrac_, minInt_ + maxFrac_).withMode(roundingMode); + int maxSig_ = minInt_ + maxFrac_; + // Bug #20058: if maxInt_ > minInt_ > 1, then minInt_ should be 1. + if (maxInt_ > minInt_ && minInt_ > 1) { + minInt_ = 1; + } + int minSig_ = minInt_ + minFrac_; + // To avoid regression, maxSig is not reset when minInt_ set to 1. + // TODO: Reset maxSig_ = 1 + minFrac_ to follow the spec. + macros.precision = Precision::constructSignificant(minSig_, maxSig_).withMode(roundingMode); } } } diff --git a/icu4c/source/test/testdata/numberformattestspecification.txt b/icu4c/source/test/testdata/numberformattestspecification.txt index 86076370393..66bff3c0a63 100644 --- a/icu4c/source/test/testdata/numberformattestspecification.txt +++ b/icu4c/source/test/testdata/numberformattestspecification.txt @@ -353,6 +353,15 @@ minIntegerDigits maxIntegerDigits minFractionDigits maxFractionDigits output bre // JDK fails here because it tries to use 9 + 6 = 15 sig digits. 2 9 1 6 29.979246E7 K +test ticket 20058 +set locale en +begin +pattern format output breaks +#00.0##E0 0 0.0E0 K +#00.0##E0 1.2 1.2E0 K +#00.0E0 0 0.0E0 K +#00.0E0 1.2 1.2E0 K + test significant digits scientific set locale en set pattern #E0 diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java index 4794c94dd4c..213f7658536 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java @@ -263,8 +263,8 @@ final class NumberPropertyMapper { // TODO: Overriding here is a bit of a hack. Should this logic go earlier? if (macros.precision instanceof FractionPrecision) { // For the purposes of rounding, get the original min/max int/frac, since the local - // variables - // have been manipulated for display purposes. + // variables have been manipulated for display purposes. + int maxInt_ = properties.getMaximumIntegerDigits(); int minInt_ = properties.getMinimumIntegerDigits(); int minFrac_ = properties.getMinimumFractionDigits(); int maxFrac_ = properties.getMaximumFractionDigits(); @@ -275,8 +275,15 @@ final class NumberPropertyMapper { // Patterns like "#.##E0" (no zeros in the mantissa), which mean round to maxFrac+1 macros.precision = Precision.constructSignificant(1, maxFrac_ + 1).withMode(mathContext); } else { - // All other scientific patterns, which mean round to minInt+maxFrac - macros.precision = Precision.constructSignificant(minInt_ + minFrac_, minInt_ + maxFrac_) + int maxSig_ = minInt_ + maxFrac_; + // Bug #20058: if maxInt_ > minInt_ > 1, then minInt_ should be 1. + if (maxInt_ > minInt_ && minInt_ > 1) { + minInt_ = 1; + } + int minSig_ = minInt_ + minFrac_; + // To avoid regression, maxSig is not reset when minInt_ set to 1. + // TODO: Reset maxSig_ = 1 + minFrac_ to follow the spec. + macros.precision = Precision.constructSignificant(minSig_, maxSig_) .withMode(mathContext); } } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/data/numberformattestspecification.txt b/icu4j/main/tests/core/src/com/ibm/icu/dev/data/numberformattestspecification.txt index 86076370393..66bff3c0a63 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/data/numberformattestspecification.txt +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/data/numberformattestspecification.txt @@ -353,6 +353,15 @@ minIntegerDigits maxIntegerDigits minFractionDigits maxFractionDigits output bre // JDK fails here because it tries to use 9 + 6 = 15 sig digits. 2 9 1 6 29.979246E7 K +test ticket 20058 +set locale en +begin +pattern format output breaks +#00.0##E0 0 0.0E0 K +#00.0##E0 1.2 1.2E0 K +#00.0E0 0 0.0E0 K +#00.0E0 1.2 1.2E0 K + test significant digits scientific set locale en set pattern #E0 -- 2.40.0