From cbae6dfbaa1dba01154b27056eaadbc82eba621d Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Sat, 14 Apr 2018 07:15:19 +0000 Subject: [PATCH] ICU-13634 Adding groupingUsed as an explicit property in the property bag; see also ICU-13442 . X-SVN-Rev: 41229 --- icu4c/source/i18n/decimfmt.cpp | 18 +++++----- icu4c/source/i18n/number_decimfmtprops.cpp | 2 ++ icu4c/source/i18n/number_decimfmtprops.h | 1 + icu4c/source/i18n/number_grouping.cpp | 3 ++ icu4c/source/i18n/number_patternstring.cpp | 2 ++ icu4c/source/test/intltest/dcfmapts.cpp | 36 +++++++++++++++---- .../impl/number/DecimalFormatProperties.java | 21 +++++++++++ .../src/com/ibm/icu/impl/number/Grouper.java | 3 ++ .../icu/impl/number/PatternStringParser.java | 2 ++ .../src/com/ibm/icu/text/DecimalFormat.java | 14 +++----- .../format/NumberFormatRegressionTest.java | 28 +++++++++++++++ .../icu/dev/test/format/NumberFormatTest.java | 9 +---- 12 files changed, 107 insertions(+), 32 deletions(-) diff --git a/icu4c/source/i18n/decimfmt.cpp b/icu4c/source/i18n/decimfmt.cpp index b3bc2455d49..46974b36651 100644 --- a/icu4c/source/i18n/decimfmt.cpp +++ b/icu4c/source/i18n/decimfmt.cpp @@ -308,14 +308,7 @@ int32_t DecimalFormat::getAttribute(UNumberFormatAttribute attr, UErrorCode& sta void DecimalFormat::setGroupingUsed(UBool enabled) { NumberFormat::setGroupingUsed(enabled); // to set field for compatibility - if (enabled) { - // Set to a reasonable default value - fProperties->groupingSize = 3; - fProperties->secondaryGroupingSize = -1; - } else { - fProperties->groupingSize = 0; - fProperties->secondaryGroupingSize = 0; - } + fProperties->groupingUsed = enabled; refreshFormatterNoError(); } @@ -346,7 +339,7 @@ DecimalFormat::DecimalFormat(const UnicodeString& pattern, const DecimalFormatSy refreshFormatter(status); } -DecimalFormat::DecimalFormat(const DecimalFormat& source) { +DecimalFormat::DecimalFormat(const DecimalFormat& source) : NumberFormat(source) { fProperties.adoptInstead(new DecimalFormatProperties(*source.fProperties)); fExportedProperties.adoptInstead(new DecimalFormatProperties()); fWarehouse.adoptInstead(new DecimalFormatWarehouse()); @@ -501,6 +494,7 @@ void DecimalFormat::parse(const UnicodeString& text, Formattable& output, // parseCurrency method (backwards compatibility) int32_t startIndex = parsePosition.getIndex(); fParser->parse(text, startIndex, true, result, status); + // TODO: Do we need to check for fProperties->parseAllInput (UCONFIG_HAVE_PARSEALLINPUT) here? if (result.success()) { parsePosition.setIndex(result.charEnd); result.populateFormattable(output); @@ -520,6 +514,7 @@ CurrencyAmount* DecimalFormat::parseCurrency(const UnicodeString& text, ParsePos // parseCurrency method (backwards compatibility) int32_t startIndex = parsePosition.getIndex(); fParserWithCurrency->parse(text, startIndex, true, result, status); + // TODO: Do we need to check for fProperties->parseAllInput (UCONFIG_HAVE_PARSEALLINPUT) here? if (result.success()) { parsePosition.setIndex(result.charEnd); Formattable formattable; @@ -745,6 +740,9 @@ void DecimalFormat::setExponentSignAlwaysShown(UBool expSignAlways) { } int32_t DecimalFormat::getGroupingSize(void) const { + if (fProperties->groupingSize < 0) { + return 0; + } return fProperties->groupingSize; } @@ -1038,6 +1036,8 @@ void DecimalFormat::refreshFormatter(UErrorCode& status) { NumberFormat::setMinimumIntegerDigits(fExportedProperties->minimumIntegerDigits); NumberFormat::setMaximumFractionDigits(fExportedProperties->maximumFractionDigits); NumberFormat::setMinimumFractionDigits(fExportedProperties->minimumFractionDigits); + // fProperties, not fExportedProperties, since this information comes from the pattern: + NumberFormat::setGroupingUsed(fProperties->groupingUsed); } void DecimalFormat::refreshFormatterNoError() { diff --git a/icu4c/source/i18n/number_decimfmtprops.cpp b/icu4c/source/i18n/number_decimfmtprops.cpp index 54fd6a1ef17..f7d486d699e 100644 --- a/icu4c/source/i18n/number_decimfmtprops.cpp +++ b/icu4c/source/i18n/number_decimfmtprops.cpp @@ -25,6 +25,7 @@ void DecimalFormatProperties::clear() { exponentSignAlwaysShown = false; formatWidth = -1; groupingSize = -1; + groupingUsed = false; magnitudeMultiplier = 0; maximumFractionDigits = -1; maximumIntegerDigits = -1; @@ -68,6 +69,7 @@ bool DecimalFormatProperties::operator==(const DecimalFormatProperties &other) c eq = eq && exponentSignAlwaysShown == other.exponentSignAlwaysShown; eq = eq && formatWidth == other.formatWidth; eq = eq && groupingSize == other.groupingSize; + eq = eq && groupingUsed == other.groupingUsed; eq = eq && magnitudeMultiplier == other.magnitudeMultiplier; eq = eq && maximumFractionDigits == other.maximumFractionDigits; eq = eq && maximumIntegerDigits == other.maximumIntegerDigits; diff --git a/icu4c/source/i18n/number_decimfmtprops.h b/icu4c/source/i18n/number_decimfmtprops.h index d66a621250c..4ad3225ba39 100644 --- a/icu4c/source/i18n/number_decimfmtprops.h +++ b/icu4c/source/i18n/number_decimfmtprops.h @@ -98,6 +98,7 @@ struct U_I18N_API DecimalFormatProperties { bool exponentSignAlwaysShown; int32_t formatWidth; int32_t groupingSize; + bool groupingUsed; int32_t magnitudeMultiplier; int32_t maximumFractionDigits; int32_t maximumIntegerDigits; diff --git a/icu4c/source/i18n/number_grouping.cpp b/icu4c/source/i18n/number_grouping.cpp index 90bcc4fb5b3..7bda111046b 100644 --- a/icu4c/source/i18n/number_grouping.cpp +++ b/icu4c/source/i18n/number_grouping.cpp @@ -52,6 +52,9 @@ Grouper Grouper::forStrategy(UGroupingStrategy grouping) { } Grouper Grouper::forProperties(const DecimalFormatProperties& properties) { + if (!properties.groupingUsed) { + return forStrategy(UNUM_GROUPING_OFF); + } auto grouping1 = static_cast(properties.groupingSize); auto grouping2 = static_cast(properties.secondaryGroupingSize); auto minGrouping = static_cast(properties.minimumGroupingDigits); diff --git a/icu4c/source/i18n/number_patternstring.cpp b/icu4c/source/i18n/number_patternstring.cpp index e32ad6b130d..e96f958da0f 100644 --- a/icu4c/source/i18n/number_patternstring.cpp +++ b/icu4c/source/i18n/number_patternstring.cpp @@ -497,8 +497,10 @@ PatternParser::patternInfoToProperties(DecimalFormatProperties& properties, Pars auto grouping3 = static_cast ((positive.groupingSizes >> 32) & 0xffff); if (grouping2 != -1) { properties.groupingSize = grouping1; + properties.groupingUsed = true; } else { properties.groupingSize = -1; + properties.groupingUsed = false; } if (grouping3 != -1) { properties.secondaryGroupingSize = grouping2; diff --git a/icu4c/source/test/intltest/dcfmapts.cpp b/icu4c/source/test/intltest/dcfmapts.cpp index cf854de4068..b1bde0121ed 100644 --- a/icu4c/source/test/intltest/dcfmapts.cpp +++ b/icu4c/source/test/intltest/dcfmapts.cpp @@ -113,15 +113,39 @@ void IntlTestDecimalFormatAPI::testAPI(/*char *par*/) // bug 10864 status = U_ZERO_ERROR; DecimalFormat noGrouping("###0.##", status); - if (noGrouping.getGroupingSize() != 0) { - errln("Grouping size should be 0 for no grouping."); - } + assertEquals("Grouping size should be 0 for no grouping.", 0, noGrouping.getGroupingSize()); noGrouping.setGroupingUsed(TRUE); - if (noGrouping.getGroupingSize() != 0) { - errln("Grouping size should still be 0."); - } + assertEquals("Grouping size should still be 0.", 0, noGrouping.getGroupingSize()); // end bug 10864 + // bug 13442 comment 14 + status = U_ZERO_ERROR; + { + DecimalFormat df("0", {"en", status}, status); + UnicodeString result; + assertEquals("pat 0: ", 0, df.getGroupingSize()); + assertEquals("pat 0: ", (UBool) FALSE, (UBool) df.isGroupingUsed()); + df.setGroupingUsed(false); + assertEquals("pat 0 then disabled: ", 0, df.getGroupingSize()); + assertEquals("pat 0 then disabled: ", u"1111", df.format(1111, result.remove())); + df.setGroupingUsed(true); + assertEquals("pat 0 then enabled: ", 0, df.getGroupingSize()); + assertEquals("pat 0 then enabled: ", u"1111", df.format(1111, result.remove())); + } + { + DecimalFormat df("#,##0", {"en", status}, status); + UnicodeString result; + assertEquals("pat #,##0: ", 3, df.getGroupingSize()); + assertEquals("pat #,##0: ", (UBool) TRUE, (UBool) df.isGroupingUsed()); + df.setGroupingUsed(false); + assertEquals("pat #,##0 then disabled: ", 3, df.getGroupingSize()); + assertEquals("pat #,##0 then disabled: ", u"1111", df.format(1111, result.remove())); + df.setGroupingUsed(true); + assertEquals("pat #,##0 then enabled: ", 3, df.getGroupingSize()); + assertEquals("pat #,##0 then enabled: ", u"1,111", df.format(1111, result.remove())); + } + // end bug 13442 comment 14 + status = U_ZERO_ERROR; const UnicodeString pattern("#,##0.# FF"); DecimalFormat pat(pattern, status); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalFormatProperties.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalFormatProperties.java index d89f62556d0..e859e576f75 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalFormatProperties.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalFormatProperties.java @@ -87,6 +87,7 @@ public class DecimalFormatProperties implements Cloneable, Serializable { private transient boolean exponentSignAlwaysShown; private transient int formatWidth; private transient int groupingSize; + private transient boolean groupingUsed; private transient int magnitudeMultiplier; private transient MathContext mathContext; // ICU4J-only private transient int maximumFractionDigits; @@ -158,6 +159,7 @@ public class DecimalFormatProperties implements Cloneable, Serializable { exponentSignAlwaysShown = false; formatWidth = -1; groupingSize = -1; + groupingUsed = false; magnitudeMultiplier = 0; mathContext = null; maximumFractionDigits = -1; @@ -203,6 +205,7 @@ public class DecimalFormatProperties implements Cloneable, Serializable { exponentSignAlwaysShown = other.exponentSignAlwaysShown; formatWidth = other.formatWidth; groupingSize = other.groupingSize; + groupingUsed = other.groupingUsed; magnitudeMultiplier = other.magnitudeMultiplier; mathContext = other.mathContext; maximumFractionDigits = other.maximumFractionDigits; @@ -249,6 +252,7 @@ public class DecimalFormatProperties implements Cloneable, Serializable { eq = eq && _equalsHelper(exponentSignAlwaysShown, other.exponentSignAlwaysShown); eq = eq && _equalsHelper(formatWidth, other.formatWidth); eq = eq && _equalsHelper(groupingSize, other.groupingSize); + eq = eq && _equalsHelper(groupingUsed, other.groupingUsed); eq = eq && _equalsHelper(magnitudeMultiplier, other.magnitudeMultiplier); eq = eq && _equalsHelper(mathContext, other.mathContext); eq = eq && _equalsHelper(maximumFractionDigits, other.maximumFractionDigits); @@ -311,6 +315,7 @@ public class DecimalFormatProperties implements Cloneable, Serializable { hashCode ^= _hashCodeHelper(exponentSignAlwaysShown); hashCode ^= _hashCodeHelper(formatWidth); hashCode ^= _hashCodeHelper(groupingSize); + hashCode ^= _hashCodeHelper(groupingUsed); hashCode ^= _hashCodeHelper(magnitudeMultiplier); hashCode ^= _hashCodeHelper(mathContext); hashCode ^= _hashCodeHelper(maximumFractionDigits); @@ -439,6 +444,10 @@ public class DecimalFormatProperties implements Cloneable, Serializable { return groupingSize; } + public boolean getGroupingUsed() { + return groupingUsed; + } + public int getMagnitudeMultiplier() { return magnitudeMultiplier; } @@ -790,6 +799,18 @@ public class DecimalFormatProperties implements Cloneable, Serializable { return this; } + /** + * Sets whether to enable grouping when formatting. + * + * @param groupingUsed + * true to enable the display of grouping separators; false to disable. + * @return The property bag, for chaining. + */ + public DecimalFormatProperties setGroupingUsed(boolean groupingUsed) { + this.groupingUsed = groupingUsed; + return this; + } + /** * Multiply all numbers by this power of ten before formatting. Negative multipliers reduce the * magnitude and make numbers smaller (closer to zero). diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/Grouper.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/Grouper.java index 6e18907dbbc..816a7e3f10f 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/Grouper.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/Grouper.java @@ -48,6 +48,9 @@ public class Grouper { * Resolve the values in Properties to a Grouper object. */ public static Grouper forProperties(DecimalFormatProperties properties) { + if (!properties.getGroupingUsed()) { + return GROUPER_NEVER; + } short grouping1 = (short) properties.getGroupingSize(); short grouping2 = (short) properties.getSecondaryGroupingSize(); short minGrouping = (short) properties.getMinimumGroupingDigits(); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PatternStringParser.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PatternStringParser.java index 53dd482baab..3d6c25ddb73 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PatternStringParser.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PatternStringParser.java @@ -559,8 +559,10 @@ public class PatternStringParser { short grouping3 = (short) ((positive.groupingSizes >>> 32) & 0xffff); if (grouping2 != -1) { properties.setGroupingSize(grouping1); + properties.setGroupingUsed(true); } else { properties.setGroupingSize(-1); + properties.setGroupingUsed(false); } if (grouping3 != -1) { properties.setSecondaryGroupingSize(grouping2); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormat.java index 6c31196189f..e7ddd465790 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormat.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormat.java @@ -1844,7 +1844,7 @@ public class DecimalFormat extends NumberFormat { */ @Override public synchronized boolean isGroupingUsed() { - return properties.getGroupingSize() > 0 || properties.getSecondaryGroupingSize() > 0; + return properties.getGroupingUsed(); } /** @@ -1866,14 +1866,7 @@ public class DecimalFormat extends NumberFormat { */ @Override public synchronized void setGroupingUsed(boolean enabled) { - if (enabled) { - // Set to a reasonable default value - properties.setGroupingSize(3); - properties.setSecondaryGroupingSize(-1); - } else { - properties.setGroupingSize(0); - properties.setSecondaryGroupingSize(0); - } + properties.setGroupingUsed(enabled); refreshFormatter(); } @@ -1885,6 +1878,9 @@ public class DecimalFormat extends NumberFormat { * @stable ICU 2.0 */ public synchronized int getGroupingSize() { + if (properties.getGroupingSize() < 0) { + return 0; + } return properties.getGroupingSize(); } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatRegressionTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatRegressionTest.java index c07161b10a0..705f5c04620 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatRegressionTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatRegressionTest.java @@ -397,4 +397,32 @@ public class NumberFormatRegressionTest extends TestFmwk { " (unknown currency)", nf.getPositiveSuffix()); } + + @Test + public void TestGroupingEnabledDisabledGetters() { + // See ticket #13442 comment 14 + DecimalFormatSymbols EN = DecimalFormatSymbols.getInstance(ULocale.ENGLISH); + { + DecimalFormat df = new DecimalFormat("0", EN); + assertEquals("pat 0: ", 0, df.getGroupingSize()); + assertEquals("pat 0: ", false, df.isGroupingUsed()); + df.setGroupingUsed(false); + assertEquals("pat 0 then disabled: ", 0, df.getGroupingSize()); + assertEquals("pat 0 then disabled: ", "1111", df.format(1111)); + df.setGroupingUsed(true); + assertEquals("pat 0 then enabled: ", 0, df.getGroupingSize()); + assertEquals("pat 0 then enabled: ", "1111", df.format(1111)); + } + { + DecimalFormat df = new DecimalFormat("#,##0", EN); + assertEquals("pat #,##0: ", 3, df.getGroupingSize()); + assertEquals("pat #,##0: ", true, df.isGroupingUsed()); + df.setGroupingUsed(false); + assertEquals("pat #,##0 then disabled: ", 3, df.getGroupingSize()); + assertEquals("pat #,##0 then disabled: ", "1111", df.format(1111)); + df.setGroupingUsed(true); + assertEquals("pat #,##0 then enabled: ", 3, df.getGroupingSize()); + assertEquals("pat #,##0 then enabled: ", "1,111", df.format(1111)); + } + } } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java index de6c3927779..4b63eab0982 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java @@ -2838,6 +2838,7 @@ public class NumberFormatTest extends TestFmwk { // Grouping separators are not allowed in the pattern, but we can enable them via the API. DecimalFormat df = new DecimalFormat("###0.000E0"); df.setGroupingUsed(true); + df.setGroupingSize(3); expect2(df, 123, "123.0E0"); expect2(df, 1234, "1,234E0"); expect2(df, 12340, "1.234E4"); @@ -5373,14 +5374,6 @@ public class NumberFormatTest extends TestFmwk { 130, resultScientific.longValue()); } - @Test - public void Test13442() { - DecimalFormat df = new DecimalFormat(); - df.setGroupingUsed(false); - assertEquals("Grouping size should now be zero", 0, df.getGroupingSize()); - assertEquals("Grouping should be off", false, df.isGroupingUsed()); - } - @Test public void Test13453_AffixContent() { DecimalFormat df = (DecimalFormat) DecimalFormat.getScientificInstance();