ICU-13634 Adding groupingUsed as an explicit property in the property bag; see also...

X-SVN-Rev: 41229

diff --git a/icu4c/source/i18n/decimfmt.cpp b/icu4c/source/i18n/decimfmt.cpp
index b3bc2455d49336895c74f612961172e7bed631da..46974b3665144957b2507ada029c59a00ab7d047 100644 (file)
--- 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 54fd6a1ef17e702e3667c21dce2129e99196a36c..f7d486d699e43a3e1ae6a1bf21b6cee14bd4bdec 100644 (file)
--- 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 d66a621250ced136d80236a5f9c4ba4bbcf5e9c6..4ad3225ba39c8c75824b64dddc24fb6c5e3b4c71 100644 (file)
--- 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 90bcc4fb5b39f42d31b5ec1e1428b1ce8b01b9f2..7bda111046b0c997acc88b399d0420727ff955f1 100644 (file)
--- 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<int16_t>(properties.groupingSize);
     auto grouping2 = static_cast<int16_t>(properties.secondaryGroupingSize);
     auto minGrouping = static_cast<int16_t>(properties.minimumGroupingDigits);

diff --git a/icu4c/source/i18n/number_patternstring.cpp b/icu4c/source/i18n/number_patternstring.cpp
index e32ad6b130ddafa44e480a3f49b3651ddf729efa..e96f958da0faaed0a19dcbf777ab1b068ef5d76b 100644 (file)
--- 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<int16_t> ((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 cf854de4068acdeaf4d6b17b4b554cf609169e81..b1bde0121ed3e50f6eab8b27657863650f5ab38f 100644 (file)
--- 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 d89f62556d04948595e92d14c35c220142f64310..e859e576f75cfdc8956c986fc1e20cbb9883f5ca 100644 (file)
--- 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 6e18907dbbc13d9b4a44d1be86c7d2409e8a5ab2..816a7e3f10fa3ddc09b842bcd9cc7930b88c7d34 100644 (file)
--- 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 53dd482baab76640b2f02e67130e4e01d4a74a88..3d6c25ddb737f7ca10c34afa021c1ab9e4e87248 100644 (file)
--- 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 6c31196189f68e09e79a89d269bb45bb56d9a028..e7ddd4657900684d4dbc3f365cb3d39d983cf5e8 100644 (file)
--- 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 c07161b10a0e8a666fa1940c96744e367aaa7b30..705f5c04620f2218df6f52cf49b256c277fd04d7 100644 (file)
--- 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 de6c39277790afb05a1307612255977ae4a80b7f..4b63eab0982a7dffe3224bf234edcd831ee510f5 100644 (file)
--- 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();