Add digit grouping to display

Bug: 27461988

Add digit grouping separators to formula and result.

The result display piece of this is an annoyingly complex change,
since it interacts with our already subtle calculations of the
number of digits to display, our initial evaluation precision,
etc.

We display grouping separators in the result only when the entire
whole part of the number, with grouping separators, fits into
the display.  If it fits into the display without, but not with,
grouping separators, we force scientific notation. This may require
the result to be scrollable when it otherwise would not be, and
leads to an interesting set of corner cases, which we claim to
handle reasonably.

Some cleanups were applied to make this easier, more useful, and
more debuggable.  These included:

More accurate bookkeeping about different character widths. Otherwise
scrolling with grouping separators was not smooth.

Ignore grouping separators and spaces on input, as we should have
been doing all along.

Only redisplay the result if the character (as opposed to pixel)
position changed. This makes up for some extra computation and
facilitates debugging.

Introduce StringUtils.java to hold digit string operations that really
don't fit anywhere else.  Move the duplicated repeat() function there.

Change-Id: I00502b9906b184671cd3379cd68b0447939b2394

1 files changed, 26 insertions, 21 deletions

diff --git a/src/com/android/calculator2/Evaluator.java b/src/com/android/calculator2/Evaluator.java
index 357b302..6667878 100644
--- a/src/com/android/calculator2/Evaluator.java
+++ b/src/com/android/calculator2/Evaluator.java
@@ -465,10 +465,10 @@ class Evaluator {
         }
         // Earlier digits could not have changed without a 0 to 9 or 9 to 0 flip at end.
         // The former is OK.
-        if (!newDigs.substring(newLen - precDiff).equals(repeat('0', precDiff))) {
+        if (!newDigs.substring(newLen - precDiff).equals(StringUtils.repeat('0', precDiff))) {
             throw new AssertionError("New approximation invalidates old one!");
         }
-        return oldDigs + repeat('9', precDiff);
+        return oldDigs + StringUtils.repeat('9', precDiff);
     }
 
     /**
@@ -582,19 +582,25 @@ class Evaluator {
     private int getPreferredPrec(String cache, int msd, int lastDigitOffset) {
         final int lineLength = mResult.getMaxChars();
         final int wholeSize = cache.indexOf('.');
+        final float rawSepChars = mResult.separatorChars(cache, wholeSize);
+        final float rawSepCharsNoDecimal = rawSepChars - mResult.getNoEllipsisCredit();
+        final float rawSepCharsWithDecimal = rawSepCharsNoDecimal - mResult.getDecimalCredit();
+        final int sepCharsNoDecimal = (int) Math.ceil(Math.max(rawSepCharsNoDecimal, 0.0f));
+        final int sepCharsWithDecimal = (int) Math.ceil(Math.max(rawSepCharsWithDecimal, 0.0f));
         final int negative = cache.charAt(0) == '-' ? 1 : 0;
         // Don't display decimal point if result is an integer.
         if (lastDigitOffset == 0) {
             lastDigitOffset = -1;
         }
         if (lastDigitOffset != Integer.MAX_VALUE) {
-            if (wholeSize <= lineLength && lastDigitOffset <= 0) {
+            if (wholeSize <= lineLength - sepCharsNoDecimal && lastDigitOffset <= 0) {
                 // Exact integer.  Prefer to display as integer, without decimal point.
                 return -1;
             }
             if (lastDigitOffset >= 0
-                    && wholeSize + lastDigitOffset + 1 /* decimal pt. */ <= lineLength) {
-                // Display full exact number wo scientific notation.
+                    && wholeSize + lastDigitOffset + 1 /* decimal pt. */
+                    <= lineLength - sepCharsWithDecimal) {
+                // Display full exact number without scientific notation.
                 return lastDigitOffset;
             }
         }
@@ -609,10 +615,20 @@ class Evaluator {
             // Treat extremely large msd values as unknown to avoid slow computations.
             return lineLength - 2;
         }
-        // Return position corresponding to having msd at left, effectively
-        // presuming scientific notation that preserves the left part of the
-        // result.
-        return msd - wholeSize + lineLength - negative - 1;
+        // Return position corresponding to having msd at left, effectively presuming scientific
+        // notation that preserves the left part of the result.
+        // After adjustment for the space required by an exponent, evaluating to the resulting
+        // precision should not overflow the display.
+        int result = msd - wholeSize + lineLength - negative - 1;
+        if (wholeSize <= lineLength - sepCharsNoDecimal) {
+            // Fits without scientific notation; will need space for separators.
+            if (wholeSize < lineLength - sepCharsWithDecimal) {
+                result -= sepCharsWithDecimal;
+            } else {
+                result -= sepCharsNoDecimal;
+            }
+        }
+        return result;
     }
 
     private static final int SHORT_TARGET_LENGTH  = 8;
@@ -745,17 +761,6 @@ class Evaluator {
         return result;
     }
 
-    /**
-     * Return a string with n copies of c.
-     */
-    private static String repeat(char c, int n) {
-        StringBuilder result = new StringBuilder();
-        for (int i = 0; i < n; ++i) {
-            result.append(c);
-        }
-        return result.toString();
-    }
-
     // Refuse to scroll past the point at which this many digits from the whole number
     // part of the result are still displayed.  Avoids sily displays like 1E1.
     private static final int MIN_DISPLAYED_DIGS = 5;
@@ -823,7 +828,7 @@ class Evaluator {
         truncated[0] = (startIndex > getMsdIndex());
         String result = mResultString.substring(startIndex, endIndex);
         if (deficit > 0) {
-            result += repeat(' ', deficit);
+            result += StringUtils.repeat(' ', deficit);
             // Blank character is replaced during translation.
             // Since we always compute past the decimal point, this never fills in the spot
             // where the decimal point should go, and we can otherwise treat placeholders