More fixes inspired by bug #48872 - handle zero formatting rules, and better color detection in DataFormatter

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@950035 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 152 insertions, 18 deletions

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 2b68341b35..b73f1ce5be 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,8 @@
 
     <changes>
         <release version="3.7-SNAPSHOT" date="2010-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">48872 - handle zero formatting rules, and better color detection in DataFormatter</action>
+           <action dev="POI-DEVELOPERS" type="fix">48872 - support for more kinds of formatting in DataFormatter</action>
            <action dev="POI-DEVELOPERS" type="fix">43161 - fixed construction of the DIB picture header</action>
            <action dev="POI-DEVELOPERS" type="add">49311 - initial support for reading AES-encrypted/write-protected OOXML files</action>
            <action dev="POI-DEVELOPERS" type="fix">48718 - Make the creation of multiple, un-modified fonts in a row in XSSF match the old HSSF behaviour</action>
diff --git a/src/java/org/apache/poi/ss/usermodel/DataFormatter.java b/src/java/org/apache/poi/ss/usermodel/DataFormatter.java
index 053e37cb50..bef1b69d31 100644
--- a/src/java/org/apache/poi/ss/usermodel/DataFormatter.java
+++ b/src/java/org/apache/poi/ss/usermodel/DataFormatter.java
@@ -77,6 +77,16 @@ public class DataFormatter {
     /** A regex to find patterns like [$$-1009] and [$?-452]. */
     private static final Pattern specialPatternGroup = Pattern.compile("(\\[\\$[^-\\]]*-[0-9A-Z]+\\])");
 
+    /** 
+     * A regex to match the colour formattings rules.
+     * Allowed colours are: Black, Blue, Cyan, Green,
+     *  Magenta, Red, White, Yellow, "Color n" (1<=n<=56)
+     */
+    private static final Pattern colorPattern = 
+       Pattern.compile("(\\[BLACK\\])|(\\[BLUE\\])|(\\[CYAN\\])|(\\[GREEN\\])|" +
+       		"(\\[MAGENTA\\])|(\\[RED\\])|(\\[WHITE\\])|(\\[YELLOW\\])|" +
+       		"(\\[COLOR\\s*\\d\\])|(\\[COLOR\\s*[0-5]\\d\\])", Pattern.CASE_INSENSITIVE);
+    
     /**
      * The decimal symbols of the locale used for formatting values.
      */
@@ -165,6 +175,20 @@ public class DataFormatter {
     }
 
     private Format getFormat(double cellValue, int formatIndex, String formatStr) {
+       // Excel supports positive/negative/zero, but java
+       // doesn't, so we need to do it specially
+       if(formatStr.indexOf(';') != formatStr.lastIndexOf(';')) {
+          int lastAt = formatStr.lastIndexOf(';');
+          String zeroFormat = formatStr.substring(lastAt+1);
+          String normalFormat = formatStr.substring(0,lastAt);
+          if(cellValue == 0.0) {
+             formatStr = zeroFormat;
+          } else {
+             formatStr = normalFormat;
+          }
+       }
+      
+       // See if we already have it cached
         Format format = formats.get(formatStr);
         if (format != null) {
             return format;
@@ -195,9 +219,25 @@ public class DataFormatter {
     }
 
     private Format createFormat(double cellValue, int formatIndex, String sFormat) {
-        // remove color formatting if present
-        String formatStr = sFormat.replaceAll("\\[[a-zA-Z]*\\]", "");
-
+        String formatStr = sFormat;
+        
+        // Remove colour formatting if present
+        Matcher colourM = colorPattern.matcher(formatStr);
+        while(colourM.find()) {
+           String colour = colourM.group();
+           
+           // Paranoid replacement...
+           int at = formatStr.indexOf(colour);
+           if(at == -1) break;
+           String nFormatStr = formatStr.substring(0,at) +
+              formatStr.substring(at+colour.length()+1);
+           if(nFormatStr.equals(formatStr)) break;
+
+           // Try again in case there's multiple
+           formatStr = nFormatStr;
+           colourM = colorPattern.matcher(formatStr);
+        }
+        
         // try to extract special characters like currency
         Matcher m = specialPatternGroup.matcher(formatStr);
         while(m.find()) {
@@ -329,23 +369,33 @@ public class DataFormatter {
 
     private Format createNumberFormat(String formatStr, double cellValue) {
         StringBuffer sb = new StringBuffer(formatStr);
-        for (int i = 0; i < sb.length(); i++) {
+        
+        // If they requested spacers, with "_",
+        //  remove those as we don't do spacing
+        for(int i = 0; i < sb.length(); i++) {
             char c = sb.charAt(i);
-            //handle (#,##0_);
-            if (c == '(') {
-                int idx = sb.indexOf(")", i);
-                if (idx > -1 && sb.charAt(idx -1) == '_') {
-                    sb.deleteCharAt(idx);
-                    sb.deleteCharAt(idx - 1);
-                    sb.deleteCharAt(i);
-                    i--;
-                }
-            } else if (c == ')' && i > 0 && sb.charAt(i - 1) == '_') {
-                sb.deleteCharAt(i);
-                sb.deleteCharAt(i - 1);
-                i--;
+            if(c == '_') {
+               if(i > 0 && sb.charAt((i-1)) == '\\') {
+                  // It's escaped, don't worry
+                  continue;
+               } else {
+                  if(i < sb.length()-1) {
+                     // Remove the character we're supposed
+                     //  to match the space of
+                     sb.deleteCharAt(i+1);
+                  }
+                  // Remove the _ too
+                  sb.deleteCharAt(i);
+               }
+            }
+        }
+        
+        // Now, handle the other aspects like 
+        //  quoting and scientific notation
+        for(int i = 0; i < sb.length(); i++) {
+           char c = sb.charAt(i);
             // remove quotes and back slashes
-            } else if (c == '\\' || c == '"') {
+            if (c == '\\' || c == '"') {
                 sb.deleteCharAt(i);
                 i--;
 
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormatter.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormatter.java
index 5006440b0a..6453699892 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormatter.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormatter.java
@@ -91,6 +91,7 @@ public final class TestHSSFDataFormatter extends TestCase {
 				"0000.00000%",
 				"0.000E+00",
 				"0.00E+00",
+				"[BLACK]0.00;[COLOR 5]##.##",
 		};
 
 		// invalid date formats -- will throw exception in DecimalFormat ctor
diff --git a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java
index 73c15e8c44..51c4848475 100644
--- a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java
@@ -19,11 +19,15 @@ package org.apache.poi.ss.usermodel;
 
 import java.util.Locale;
 
+import org.apache.poi.hssf.usermodel.TestHSSFDataFormatter;
+
 import junit.framework.TestCase;
 
 /**
  * Tests of {@link DataFormatter}
  *
+ * See {@link TestHSSFDataFormatter} too for 
+ *  more tests.
  */
 public class TestDataFormatter extends TestCase {
     /**
@@ -40,4 +44,81 @@ public class TestDataFormatter extends TestCase {
        assertEquals("12.34", dfUS.formatRawCellContents(12.34, -1, "@"));
        assertEquals("12,34", dfFR.formatRawCellContents(12.34, -1, "@"));
     }
+    
+    /**
+     * Ensure that colours get correctly
+     *  zapped from within the format strings
+     */
+    public void testColours() {
+       DataFormatter dfUS = new DataFormatter(Locale.US);
+       
+       String[] formats = new String[] {
+             "##.##",
+             "[WHITE]##.##",
+             "[BLACK]##.##;[RED]-##.##",
+             "[COLOR11]##.##;[COLOR 43]-##.00",
+       };
+       for(String format : formats) {
+          assertEquals(
+                "Wrong format for: " + format,
+                "12.34",
+                dfUS.formatRawCellContents(12.343, -1, format)
+          );
+          assertEquals(
+                "Wrong format for: " + format,
+                "-12.34",
+                dfUS.formatRawCellContents(-12.343, -1, format)
+          );
+       }
+       
+       // Ensure that random square brackets remain
+       assertEquals("12.34[a]", dfUS.formatRawCellContents(12.343, -1, "##.##[a]"));
+       assertEquals("[ab]12.34[x]", dfUS.formatRawCellContents(12.343, -1, "[ab]##.##[x]"));
+    }
+    
+    /**
+     * Test how we handle negative and zeros.
+     * Note - some tests are disabled as DecimalFormat
+     *  and Excel differ, and workarounds are not
+     *  yet in place for all of these
+     */
+    public void testNegativeZero() {
+       DataFormatter dfUS = new DataFormatter(Locale.US);
+       
+       String all2dp = "00.00";
+       String alln1dp = "(00.0)";
+       String p1dp_n1dp = "00.0;(00.0)";
+       String p2dp_n1dp = "00.00;(00.0)";
+       String p2dp_n1dp_z0 = "00.00;(00.0);0";
+       String all2dpTSP = "00.00_x";
+       String p2dp_n2dpTSP = "00.00_x;(00.00)_x";
+       String p2dp_n1dpTSP = "00.00_x;(00.0)_x";
+       
+       assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, all2dp));
+       assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, p2dp_n1dp));
+       assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, p2dp_n1dp_z0));
+       
+       assertEquals("(12.3)", dfUS.formatRawCellContents(12.343, -1, alln1dp));
+       assertEquals("-(12.3)", dfUS.formatRawCellContents(-12.343, -1, alln1dp));
+       assertEquals("12.3", dfUS.formatRawCellContents(12.343, -1, p1dp_n1dp));
+       assertEquals("(12.3)", dfUS.formatRawCellContents(-12.343, -1, p1dp_n1dp));
+       
+       assertEquals("-12.34", dfUS.formatRawCellContents(-12.343, -1, all2dp));
+       // TODO - fix case of negative subpattern differing from the
+       //  positive one by more than just the prefix+suffix, which
+       //  is all DecimalFormat supports...
+//       assertEquals("(12.3)", dfUS.formatRawCellContents(-12.343, -1, p2dp_n1dp));
+//       assertEquals("(12.3)", dfUS.formatRawCellContents(-12.343, -1, p2dp_n1dp_z0));
+       
+       assertEquals("00.00", dfUS.formatRawCellContents(0, -1, all2dp));
+       assertEquals("00.00", dfUS.formatRawCellContents(0, -1, p2dp_n1dp));
+       assertEquals("0", dfUS.formatRawCellContents(0, -1, p2dp_n1dp_z0));
+       
+       // Spaces are skipped
+       assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, all2dpTSP));
+       assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, p2dp_n2dpTSP));
+       assertEquals("(12.34)", dfUS.formatRawCellContents(-12.343, -1, p2dp_n2dpTSP));
+//       assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, p2dp_n1dpTSP));
+//       assertEquals("(12.3)", dfUS.formatRawCellContents(-12.343, -1, p2dp_n1dpTSP));
+    }
 }