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

9 files changed, 440 insertions, 126 deletions

diff --git a/src/com/android/calculator2/BoundedRational.java b/src/com/android/calculator2/BoundedRational.java
index f769553..e9e6f05 100644
--- a/src/com/android/calculator2/BoundedRational.java
+++ b/src/com/android/calculator2/BoundedRational.java
@@ -91,17 +91,6 @@ public class BoundedRational {
     }
 
     /**
-     * Return a string with n copies of c.
-     */
-    static String repeat(char c, int n) {
-        final StringBuilder result = new StringBuilder();
-        for (int i = 0; i < n; ++i) {
-            result.append(c);
-        }
-        return result.toString();
-    }
-
-    /*
      * Returns a truncated (rounded towards 0) representation of the result.
      * Includes n digits to the right of the decimal point.
      * @param n result precision, >= 0
@@ -110,7 +99,7 @@ public class BoundedRational {
         String digits = mNum.abs().multiply(BigInteger.TEN.pow(n)).divide(mDen.abs()).toString();
         int len = digits.length();
         if (len < n + 1) {
-            digits = repeat('0', n + 1 - len) + digits;
+            digits = StringUtils.repeat('0', n + 1 - len) + digits;
             len = n + 1;
         }
         return (signum() < 0 ? "-" : "") + digits.substring(0, len - n) + "."
diff --git a/src/com/android/calculator2/Calculator.java b/src/com/android/calculator2/Calculator.java
index 0836ebe..f4f3da7 100644
--- a/src/com/android/calculator2/Calculator.java
+++ b/src/com/android/calculator2/Calculator.java
@@ -1054,7 +1054,7 @@ public class Calculator extends Activity
     // Display full result to currently evaluated precision
     private void displayFull() {
         Resources res = getResources();
-        String msg = mResultText.getFullText() + " ";
+        String msg = mResultText.getFullText(true /* withSeparators */) + " ";
         if (mResultText.fullTextIsExact()) {
             msg += res.getString(R.string.exact);
         } else {
@@ -1082,8 +1082,13 @@ public class Calculator extends Activity
             // Clear display immediately for incomplete function name.
             switchToInput(KeyMaps.keyForChar(moreChars.charAt(current)));
         }
+        char groupingSeparator = KeyMaps.translateResult(",").charAt(0);
         while (current < len) {
             char c = moreChars.charAt(current);
+            if (Character.isSpaceChar(c) || c == groupingSeparator) {
+                ++current;
+                continue;
+            }
             int k = KeyMaps.keyForChar(c);
             if (!explicit) {
                 int expEnd;
diff --git a/src/com/android/calculator2/CalculatorExpr.java b/src/com/android/calculator2/CalculatorExpr.java
index 21e93d6..41dfe13 100644
--- a/src/com/android/calculator2/CalculatorExpr.java
+++ b/src/com/android/calculator2/CalculatorExpr.java
@@ -198,11 +198,17 @@ class CalculatorExpr {
 
         /**
          * Produce human-readable string representation of constant, as typed.
+         * We do add digit grouping separators to the whole number, even if not typed.
          * Result is internationalized.
          */
         @Override
         public String toString() {
-            String result = mWhole;
+            String result;
+            if (mExponent != 0) {
+                result = mWhole;
+            } else {
+                result = StringUtils.addCommas(mWhole, 0, mWhole.length());
+            }
             if (mSawDecimal) {
                 result += '.';
                 result += mFraction;
diff --git a/src/com/android/calculator2/CalculatorResult.java b/src/com/android/calculator2/CalculatorResult.java
index 49308b7..0baed38 100644
--- a/src/com/android/calculator2/CalculatorResult.java
+++ b/src/com/android/calculator2/CalculatorResult.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 The Android Open Source Project
+ * Copyright (C) 2016 The Android Open Source Project
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -77,12 +77,25 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
     // We use the suffix "Index" to denote a zero-based index into a string representing a
     // result.
     private int mMaxCharOffset;  // Character offset from decimal point of rightmost digit
-                                 // that should be displayed.  Essentially the same as
+                                 // that should be displayed, plus the length of any exponent
+                                 // needed to display that digit.
+                                 // Limited to MAX_RIGHT_SCROLL. Often the same as:
     private int mLsdOffset;      // Position of least-significant digit in result
     private int mLastDisplayedOffset; // Offset of last digit actually displayed after adding
                                       // exponent.
+    private boolean mWholePartFits;  // Scientific notation not needed for initial display.
+    private float mNoExponentCredit;
+                            // Fraction of digit width saved by avoiding scientific notation.
+                            // Only accessed from UI thread.
+    private boolean mAppendExponent;
+                            // The result fits entirely in the display, even with an exponent,
+                            // but not with grouping separators. Since the result is not
+                            // scrollable, and we do not add the exponent to max. scroll position,
+                            // append an exponent insteadd of replacing trailing digits.
     private final Object mWidthLock = new Object();
-                            // Protects the next two fields.
+                            // Protects the next five fields.  These fields are only
+                            // Updated by the UI thread, and read accesses by the UI thread
+                            // sometimes do not acquire the lock.
     private int mWidthConstraint = -1;
                             // Our total width in pixels minus space for ellipsis.
     private float mCharWidth = 1;
@@ -91,8 +104,15 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
                             // TODO: We're not really using a fixed width font.  But it appears
                             // to be close enough for the characters we use that the difference
                             // is not noticeable.
+    private float mGroupingSeparatorWidthRatio;
+                            // Fraction of digit width occupied by a digit separator.
+    private float mDecimalCredit;
+                            // Fraction of digit width saved by replacing digit with decimal point.
+    private float mNoEllipsisCredit;
+                            // Fraction of digit width saved by both replacing ellipsis with digit
+                            // and avoiding scientific notation.
     private static final int MAX_WIDTH = 100;
-                            // Maximum number of digits displayed
+                            // Maximum number of digits displayed.
     public static final int MAX_LEADING_ZEROES = 6;
                             // Maximum number of leading zeroes after decimal point before we
                             // switch to scientific notation with negative exponent.
@@ -219,29 +239,104 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
         final Context context = getContext();
         final float newCharWidth = getMaxDigitWidth(paint);
         // Digits are presumed to have no more than newCharWidth.
-        // We sometimes replace a character by an ellipsis or, due to SCI_NOTATION_EXTRA, add
-        // an extra decimal separator beyond the maximum number of characters we normally allow.
-        // Empirically, our minus sign is also slightly wider than a digit, so we have to
-        // account for that.  We never have both an ellipsis and two minus signs, and
-        // we assume an ellipsis is no narrower than a minus sign.
+        // There are two instances when we know that the result is otherwise narrower than
+        // expected:
+        // 1. For standard scientific notation (our type 1), we know that we have a norrow decimal
+        // point and no (usually wide) ellipsis symbol. We allow one extra digit
+        // (SCI_NOTATION_EXTRA) to compensate, and consider that in determining available width.
+        // 2. If we are using digit grouping separators and a decimal point, we give ourselves
+        // a fractional extra space for those separators, the value of which depends on whether
+        // there is also an ellipsis.
+        //
+        // Maximum extra space we need in various cases:
+        // Type 1 scientific notation, assuming ellipsis, minus sign and E are wider than a digit:
+        //    Two minus signs + "E" + "." - 3 digits.
+        // Type 2 scientific notation:
+        //    Ellipsis + "E" + "-" - 3 digits.
+        // In the absence of scientific notation, we may need a little less space.
+        // We give ourselves a bit of extra credit towards comma insertion and give
+        // ourselves more if we have either
+        //    No ellipsis, or
+        //    A decimal separator.
+
+        // Calculate extra space we need to reserve, in addition to character count.
         final float decimalSeparatorWidth = Layout.getDesiredWidth(
                 context.getString(R.string.dec_point), paint);
-        final float minusExtraWidth = Layout.getDesiredWidth(
-                context.getString(R.string.op_sub), paint) - newCharWidth;
-        final float ellipsisExtraWidth = Layout.getDesiredWidth(KeyMaps.ELLIPSIS, paint)
-                - newCharWidth;
-        final int extraWidth = (int) (Math.ceil(Math.max(decimalSeparatorWidth + minusExtraWidth,
-                ellipsisExtraWidth)) + Math.max(minusExtraWidth, 0.0f));
+        final float minusWidth = Layout.getDesiredWidth(context.getString(R.string.op_sub), paint);
+        final float minusExtraWidth = Math.max(minusWidth - newCharWidth, 0.0f);
+        final float ellipsisWidth = Layout.getDesiredWidth(KeyMaps.ELLIPSIS, paint);
+        final float ellipsisExtraWidth =  Math.max(ellipsisWidth - newCharWidth, 0.0f);
+        final float expWidth = Layout.getDesiredWidth(KeyMaps.translateResult("e"), paint);
+        final float expExtraWidth =  Math.max(expWidth - newCharWidth, 0.0f);
+        final float type1Extra = 2 * minusExtraWidth + expExtraWidth + decimalSeparatorWidth;
+        final float type2Extra = ellipsisExtraWidth + expExtraWidth + minusExtraWidth;
+        final float extraWidth = Math.max(type1Extra, type2Extra);
+        final int intExtraWidth = (int) Math.ceil(extraWidth) + 1 /* to cover rounding sins */;
         final int newWidthConstraint = MeasureSpec.getSize(widthMeasureSpec)
-                - (getPaddingLeft() + getPaddingRight()) - extraWidth;
+                - (getPaddingLeft() + getPaddingRight()) - intExtraWidth;
+
+        // Calculate other width constants we need to handle grouping separators.
+        final float groupingSeparatorW =
+                Layout.getDesiredWidth(KeyMaps.translateResult(","), paint);
+        // Credits in the absence of any scientific notation:
+        float noExponentCredit = extraWidth - Math.max(ellipsisExtraWidth, minusExtraWidth);
+        final float noEllipsisCredit = extraWidth - minusExtraWidth;  // includes noExponentCredit.
+        final float decimalCredit = Math.max(newCharWidth - decimalSeparatorWidth, 0.0f);
+
+        mNoExponentCredit = noExponentCredit / newCharWidth;
         synchronized(mWidthLock) {
             mWidthConstraint = newWidthConstraint;
             mCharWidth = newCharWidth;
+            mNoEllipsisCredit = noEllipsisCredit / newCharWidth;
+            mDecimalCredit = decimalCredit / newCharWidth;
+            mGroupingSeparatorWidthRatio = groupingSeparatorW / newCharWidth;
         }
 
         super.onMeasure(widthMeasureSpec, heightMeasureSpec);
     }
 
+    /**
+     * Return the number of additional digit widths required to add digit separators to
+     * the supplied string prefix.
+     * The string prefix is assumed to represent a whole number, after skipping leading non-digits.
+     * Callable from non-UI thread.
+     */
+    public float separatorChars(String s, int len) {
+        int start = 0;
+        while (start < len && !Character.isDigit(s.charAt(start))) {
+            ++start;
+        }
+        // We assume the rest consists of digits, and for consistency with the rest
+        // of the code, we assume all digits have width mCharWidth.
+        final int nDigits = len - start;
+        // We currently insert a digit separator every three digits.
+        final int nSeparators = (nDigits - 1) / 3;
+        synchronized(mWidthLock) {
+            // Always return an upper bound, even in the presence of rounding errors.
+            return nSeparators * mGroupingSeparatorWidthRatio;
+        }
+    }
+
+    /**
+     * Return extra width credit for absence of ellipsis, as fraction of a digit width.
+     * May be called by non-UI thread.
+     */
+    public float getNoEllipsisCredit() {
+        synchronized(mWidthLock) {
+            return mNoEllipsisCredit;
+        }
+    }
+
+    /**
+     * Return extra width credit for presence of a decimal point, as fraction of a digit width.
+     * May be called by non-UI thread.
+     */
+    public float getDecimalCredit() {
+        synchronized(mWidthLock) {
+            return mDecimalCredit;
+        }
+    }
+
     // Return the length of the exponent representation for the given exponent, in
     // characters.
     private final int expLen(int exp) {
@@ -253,6 +348,7 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
 
     /**
      * Initiate display of a new result.
+     * Only called from UI thread.
      * The parameters specify various properties of the result.
      * @param initPrec Initial display precision computed by evaluator. (1 = tenths digit)
      * @param msd Position of most significant digit.  Offset from left of string.
@@ -274,47 +370,46 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
      * will eventually be replaced by an exponent.
      * Just appending the exponent during formatting would be simpler, but would produce
      * jumpier results during transitions.
+     * Only called from UI thread.
      */
     private void initPositions(int initPrecOffset, int msdIndex, int lsdOffset,
             String truncatedWholePart) {
-        float charWidth;
         int maxChars = getMaxChars();
+        mWholeLen = truncatedWholePart.length();
+        // Allow a tiny amount of slop for associativity/rounding differences in length
+        // calculation.  If getPreferredPrec() decided it should fit, we want to make it fit, too.
+        // We reserved one extra pixel, so the extra length is OK.
+        final int nSeparatorChars = (int) Math.ceil(
+                separatorChars(truncatedWholePart, truncatedWholePart.length())
+                - getNoEllipsisCredit() - 0.0001f);
+        mWholePartFits = mWholeLen + nSeparatorChars <= maxChars;
         mLastPos = INVALID;
         mLsdOffset = lsdOffset;
-        synchronized(mWidthLock) {
-            charWidth = mCharWidth;
-        }
-        mCurrentPos = mMinPos = (int) Math.round(initPrecOffset * charWidth);
+        mAppendExponent = false;
         // Prevent scrolling past initial position, which is calculated to show leading digits.
+        mCurrentPos = mMinPos = (int) Math.round(initPrecOffset * mCharWidth);
         if (msdIndex == Evaluator.INVALID_MSD) {
             // Possible zero value
             if (lsdOffset == Integer.MIN_VALUE) {
                 // Definite zero value.
                 mMaxPos = mMinPos;
-                mMaxCharOffset = (int) Math.round(mMaxPos/charWidth);
+                mMaxCharOffset = (int) Math.round(mMaxPos/mCharWidth);
                 mScrollable = false;
             } else {
                 // May be very small nonzero value.  Allow user to find out.
                 mMaxPos = mMaxCharOffset = MAX_RIGHT_SCROLL;
-                mMinPos -= charWidth;  // Allow for future minus sign.
+                mMinPos -= mCharWidth;  // Allow for future minus sign.
                 mScrollable = true;
             }
             return;
         }
-        mWholeLen = truncatedWholePart.length();
         int negative = truncatedWholePart.charAt(0) == '-' ? 1 : 0;
         if (msdIndex > mWholeLen && msdIndex <= mWholeLen + 3) {
             // Avoid tiny negative exponent; pretend msdIndex is just to the right of decimal point.
             msdIndex = mWholeLen - 1;
         }
+        // Set to position of leftmost significant digit relative to dec. point. Usually negative.
         int minCharOffset = msdIndex - mWholeLen;
-                                // Position of leftmost significant digit relative to dec. point.
-                                // Usually negative.
-        mMaxCharOffset = MAX_RIGHT_SCROLL; // How far does it make sense to scroll right?
-        // If msd is left of decimal point should logically be
-        // mMinPos = - (int) Math.ceil(getPaint().measureText(truncatedWholePart)), but
-        // we eventually translate to a character position by dividing by mCharWidth.
-        // To avoid rounding issues, we use the analogous computation here.
         if (minCharOffset > -1 && minCharOffset < MAX_LEADING_ZEROES + 2) {
             // Small number of leading zeroes, avoid scientific notation.
             minCharOffset = -1;
@@ -329,11 +424,16 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
             if (mMaxCharOffset < -1) {
                 currentExpLen = expLen(-minCharOffset - 1);
             } else if (minCharOffset > -1 || mMaxCharOffset >= maxChars) {
-                // Number either entirely to the right of decimal point, or decimal point not
-                // visible when scrolled to the right.
+                // Number is either entirely to the right of decimal point, or decimal point is
+                // not visible when scrolled to the right.
                 currentExpLen = expLen(-minCharOffset);
             }
-            mScrollable = (mMaxCharOffset + currentExpLen - minCharOffset + negative >= maxChars);
+            // Exponent length does not included added decimal point.  But whenever we add a
+            // decimal point, we allow an extra character (SCI_NOTATION_EXTRA).
+            final int separatorLength = mWholePartFits && minCharOffset < -3 ? nSeparatorChars : 0;
+            mScrollable = (mMaxCharOffset + currentExpLen + separatorLength - minCharOffset
+                    + negative >= maxChars);
+            // Now adjust mMaxCharOffset for any required exponent.
             int newMaxCharOffset;
             if (currentExpLen > 0) {
                 if (mScrollable) {
@@ -346,10 +446,32 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
                     // Very unlikely; just drop exponent.
                     mMaxCharOffset = -1;
                 } else {
-                    mMaxCharOffset = newMaxCharOffset;
+                    mMaxCharOffset = Math.min(newMaxCharOffset, MAX_RIGHT_SCROLL);
                 }
+                mMaxPos = Math.min((int) Math.round(mMaxCharOffset * mCharWidth),
+                        MAX_RIGHT_SCROLL);
+            } else if (!mWholePartFits && !mScrollable) {
+                // Corner case in which entire number fits, but not with grouping separators.  We
+                // will use an exponent in un-scrolled position, which may hide digits.  Scrolling
+                // by one character will remove the exponent and reveal the last digits.  Note
+                // that in the forced scientific notation case, the exponent length is not
+                // factored into mMaxCharOffset, since we do not want such an increase to impact
+                // scrolling behavior.  In the unscrollable case, we thus have to append the
+                // exponent at the end using the forcePrecision argument to formatResult, in order
+                // to ensure that we get the entire result.
+                mScrollable = (mMaxCharOffset + expLen(-minCharOffset - 1) - minCharOffset
+                        + negative >= maxChars);
+                if (mScrollable) {
+                    mMaxPos = (int) Math.ceil(mMinPos + mCharWidth);
+                    // Single character scroll will remove exponent and show remaining piece.
+                } else {
+                    mMaxPos = mMinPos;
+                    mAppendExponent = true;
+                }
+            } else {
+                mMaxPos = Math.min((int) Math.round(mMaxCharOffset * mCharWidth),
+                        MAX_RIGHT_SCROLL);
             }
-            mMaxPos = Math.min((int) Math.round(mMaxCharOffset * charWidth), MAX_RIGHT_SCROLL);
             if (!mScrollable) {
                 // Position the number consistently with our assumptions to make sure it
                 // actually fits.
@@ -361,18 +483,18 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
         }
     }
 
+    /**
+     * Display error message indicated by resourceId.
+     * UI thread only.
+     */
     void displayError(int resourceId) {
         mValid = true;
         mScrollable = false;
         final String msg = getContext().getString(resourceId);
-        final float widthConstraint;
-        synchronized(mWidthLock) {
-            widthConstraint = mWidthConstraint;
-        }
         final float measuredWidth = Layout.getDesiredWidth(msg, getPaint());
-        if (measuredWidth > widthConstraint) {
+        if (measuredWidth > mWidthConstraint) {
             // Multiply by .99 to avoid rounding effects.
-            final float scaleFactor = 0.99f * widthConstraint / measuredWidth;
+            final float scaleFactor = 0.99f * mWidthConstraint / measuredWidth;
             final RelativeSizeSpan smallTextSpan = new RelativeSizeSpan(scaleFactor);
             final SpannableString scaledMsg = new SpannableString(msg);
             scaledMsg.setSpan(smallTextSpan, 0, msg.length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
@@ -387,6 +509,7 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
     /*
      * Return the most significant digit position in the given string or Evaluator.INVALID_MSD.
      * Unlike Evaluator.getMsdIndexOf, we treat a final 1 as significant.
+     * Pure function; callable from anywhere.
      */
     public static int getNaiveMsdIndexOf(String s) {
         final int len = s.length();
@@ -399,30 +522,47 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
         return Evaluator.INVALID_MSD;
     }
 
-    // Format a result returned by Evaluator.getString() into a single line containing ellipses
-    // (if appropriate) and an exponent (if appropriate).  precOffset is the value that was passed
-    // to getString and thus identifies the significance of the rightmost digit.
-    // A value of 1 means the rightmost digits corresponds to tenths.
-    // maxDigs is the maximum number of characters in the result.
-    // If lastDisplayedOffset is not null, we set lastDisplayedOffset[0] to the offset of
-    // the last digit actually appearing in the display.
-    // If forcePrecision is true, we make sure that the last displayed digit corresponds to
-    // precOffset, and allow maxDigs to be exceeded in adding the exponent.
-    // We add two distinct kinds of exponents:
-    // (1) If the final result contains the leading digit we use standard scientific notation.
-    // (2) If not, we add an exponent corresponding to an interpretation of the final result as
-    //     an integer.
-    // We add an ellipsis on the left if the result was truncated.
-    // We add ellipses and exponents in a way that leaves most digits in the position they
-    // would have been in had we not done so.
-    // This minimizes jumps as a result of scrolling.  Result is NOT internationalized,
-    // uses "E" for exponent.
-    public String formatResult(String in, int precOffset, int maxDigs, boolean truncated,
-            boolean negative, int lastDisplayedOffset[], boolean forcePrecision) {
+    /**
+     * Format a result returned by Evaluator.getString() into a single line containing ellipses
+     * (if appropriate) and an exponent (if appropriate).
+     * We add two distinct kinds of exponents:
+     * (1) If the final result contains the leading digit we use standard scientific notation.
+     * (2) If not, we add an exponent corresponding to an interpretation of the final result as
+     *     an integer.
+     * We add an ellipsis on the left if the result was truncated.
+     * We add ellipses and exponents in a way that leaves most digits in the position they
+     * would have been in had we not done so. This minimizes jumps as a result of scrolling.
+     * Result is NOT internationalized, uses "E" for exponent.
+     * Called only from UI thread; We sometimes omit locking for fields.
+     * @param precOffset The value that was passed to getString. Identifies the significance of
+                the rightmost digit. A value of 1 means the rightmost digits corresponds to tenths.
+     * @param maxDigs The maximum number of characters in the result
+     * @param truncated The in parameter was already truncated, beyond possibly removing the
+                minus sign.
+     * @param negative The in parameter represents a negative result. (Minus sign may be removed
+                without setting truncated.)
+     * @param lastDisplayedOffset  If not null, we set lastDisplayedOffset[0] to the offset of
+                the last digit actually appearing in the display.
+     * @param forcePrecision If true, we make sure that the last displayed digit corresponds to
+                precOffset, and allow maxDigs to be exceeded in adding the exponent and commas.
+     * @param forceSciNotation Force scientific notation. May be set because we don't have
+                space for grouping separators, but whole number otherwise fits.
+     * @param insertCommas Insert commas (literally, not internationalized) as digit separators.
+                We only ever do this for the integral part of a number, and only when no
+                exponent is displayed in the initial position. The combination of which means
+                that we only do it when no exponent is displayed.
+                We insert commas in a way that does consider the width of the actual localized digit
+                separator. Commas count towards maxDigs as the appropriate fraction of a digit.
+     */
+    private String formatResult(String in, int precOffset, int maxDigs, boolean truncated,
+            boolean negative, int lastDisplayedOffset[], boolean forcePrecision,
+            boolean forceSciNotation, boolean insertCommas) {
         final int minusSpace = negative ? 1 : 0;
         final int msdIndex = truncated ? -1 : getNaiveMsdIndexOf(in);  // INVALID_MSD is OK.
         String result = in;
+        boolean needEllipsis = false;
         if (truncated || (negative && result.charAt(0) != '-')) {
+            needEllipsis = true;
             result = KeyMaps.ELLIPSIS + result.substring(1, result.length());
             // Ellipsis may be removed again in the type(1) scientific notation case.
         }
@@ -430,12 +570,16 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
         if (lastDisplayedOffset != null) {
             lastDisplayedOffset[0] = precOffset;
         }
-        if ((decIndex == -1 || msdIndex != Evaluator.INVALID_MSD
+        if (forceSciNotation || (decIndex == -1 || msdIndex != Evaluator.INVALID_MSD
                 && msdIndex - decIndex > MAX_LEADING_ZEROES + 1) &&  precOffset != -1) {
-            // No decimal point displayed, and it's not just to the right of the last digit,
-            // or we should suppress leading zeroes.
+            // Either:
+            // 1) No decimal point displayed, and it's not just to the right of the last digit, or
+            // 2) we are at the front of a number whos integral part is too large to allow
+            // comma insertion, or
+            // 3) we should suppress leading zeroes.
             // Add an exponent to let the user track which digits are currently displayed.
             // Start with type (2) exponent if we dropped no digits. -1 accounts for decimal point.
+            // We currently never show digit separators together with an exponent.
             final int initExponent = precOffset > 0 ? -precOffset : -precOffset - 1;
             int exponent = initExponent;
             boolean hasPoint = false;
@@ -448,6 +592,13 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
                 // delete leading zeroes.
                 // We try to keep leading digits roughly in position, and never
                 // lengthen the result by more than SCI_NOTATION_EXTRA.
+                if (decIndex > msdIndex) {
+                    // In the forceSciNotation, we can have a decimal point in the relevant digit
+                    // range. Remove it.
+                    result = result.substring(0, decIndex)
+                            + result.substring(decIndex + 1, result.length());
+                    // msdIndex and precOffset unaffected.
+                }
                 final int resLen = result.length();
                 String fraction = result.substring(msdIndex + 1, resLen);
                 result = (negative ? "-" : "") + result.substring(msdIndex, msdIndex + 1)
@@ -491,43 +642,94 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
                 }
             }
             result = result + "E" + Integer.toString(exponent);
+        } else if (insertCommas) {
+            // Add commas to the whole number section, and then truncate on left to fit,
+            // counting commas as a fractional digit.
+            final int wholeStart = needEllipsis ? 1 : 0;
+            int orig_length = result.length();
+            final float nCommaChars;
+            if (decIndex != -1) {
+                nCommaChars = separatorChars(result, decIndex);
+                result = StringUtils.addCommas(result, wholeStart, decIndex)
+                        + result.substring(decIndex, orig_length);
+            } else {
+                nCommaChars = separatorChars(result, orig_length);
+                result = StringUtils.addCommas(result, wholeStart, orig_length);
+            }
+            if (needEllipsis) {
+                orig_length -= 1;  // Exclude ellipsis.
+            }
+            final float len = orig_length + nCommaChars;
+            int deletedChars = 0;
+            final float ellipsisCredit = getNoEllipsisCredit();
+            final float decimalCredit = getDecimalCredit();
+            final float effectiveLen = len - (decIndex == -1 ? 0 : getDecimalCredit());
+            final float ellipsisAdjustment =
+                    needEllipsis ? mNoExponentCredit : getNoEllipsisCredit();
+            // As above, we allow for a tiny amount of extra length here, for consistency with
+            // getPreferredPrec().
+            if (effectiveLen - ellipsisAdjustment > (float) (maxDigs - wholeStart) + 0.0001f
+                && !forcePrecision) {
+                float deletedWidth = 0.0f;
+                while (effectiveLen - mNoExponentCredit - deletedWidth
+                        > (float) (maxDigs - 1 /* for ellipsis */)) {
+                    if (result.charAt(deletedChars) == ',') {
+                        deletedWidth += mGroupingSeparatorWidthRatio;
+                    } else {
+                        deletedWidth += 1.0f;
+                    }
+                    deletedChars++;
+                }
+            }
+            if (deletedChars > 0) {
+                result = KeyMaps.ELLIPSIS + result.substring(deletedChars, result.length());
+            } else if (needEllipsis) {
+                result = KeyMaps.ELLIPSIS + result;
+            }
         }
         return result;
     }
 
     /**
      * Get formatted, but not internationalized, result from mEvaluator.
-     * @param precOffset requested position (1 = tenths) of last included digit.
-     * @param maxSize Maximum number of characters (more or less) in result.
-     * @param lastDisplayedOffset Zeroth entry is set to actual offset of last included digit,
+     * @param precOffset requested position (1 = tenths) of last included digit
+     * @param maxSize maximum number of characters (more or less) in result
+     * @param lastDisplayedOffset zeroth entry is set to actual offset of last included digit,
      *                            after adjusting for exponent, etc.  May be null.
      * @param forcePrecision Ensure that last included digit is at pos, at the expense
      *                       of treating maxSize as a soft limit.
+     * @param forceSciNotation Force scientific notation, even if not required by maxSize.
+     * @param insertCommas Insert commas as digit separators.
      */
     private String getFormattedResult(int precOffset, int maxSize, int lastDisplayedOffset[],
-            boolean forcePrecision) {
+            boolean forcePrecision, boolean forceSciNotation, boolean insertCommas) {
         final boolean truncated[] = new boolean[1];
         final boolean negative[] = new boolean[1];
         final int requestedPrecOffset[] = {precOffset};
         final String rawResult = mEvaluator.getString(requestedPrecOffset, mMaxCharOffset,
                 maxSize, truncated, negative);
         return formatResult(rawResult, requestedPrecOffset[0], maxSize, truncated[0], negative[0],
-                lastDisplayedOffset, forcePrecision);
+                lastDisplayedOffset, forcePrecision, forceSciNotation, insertCommas);
    }
 
     /**
      * Return entire result (within reason) up to current displayed precision.
+     * @param withSeparators  Add digit separators
      */
-    public String getFullText() {
+    public String getFullText(boolean withSeparators) {
         if (!mValid) return "";
         if (!mScrollable) return getText().toString();
         return KeyMaps.translateResult(getFormattedResult(mLastDisplayedOffset, MAX_COPY_SIZE,
-                null, true));
+                null, true /* forcePrecision */, false /* forceSciNotation */, withSeparators));
     }
 
+    /**
+     * Did the above produce a correct result?
+     * UI thread only.
+     */
     public boolean fullTextIsExact() {
-        return !mScrollable
-                || mMaxCharOffset == getCurrentCharOffset() && mMaxCharOffset != MAX_RIGHT_SCROLL;
+        return !mScrollable || (mMaxCharOffset == getCharOffset(mCurrentPos)
+                && mMaxCharOffset != MAX_RIGHT_SCROLL);
     }
 
     /**
@@ -541,13 +743,14 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
                 || mWholeLen > MAX_RECOMPUTE_DIGITS
                 || mWholeLen + mLsdOffset > MAX_RECOMPUTE_DIGITS
                 || mLsdOffset - mLastDisplayedOffset > MAX_COPY_EXTRA) {
-            return getFullText();
+            return getFullText(false /* withSeparators */);
         }
         // It's reasonable to compute and copy the exact result instead.
         final int nonNegLsdOffset = Math.max(0, mLsdOffset);
         final String rawResult = mEvaluator.getResult().toStringTruncated(nonNegLsdOffset);
         final String formattedResult = formatResult(rawResult, nonNegLsdOffset, MAX_COPY_SIZE,
-                false, rawResult.charAt(0) == '-', null, true);
+                false, rawResult.charAt(0) == '-', null, true /* forcePrecision */,
+                false /* forceSciNotation */, false /* insertCommas */);
         return KeyMaps.translateResult(formattedResult);
     }
 
@@ -577,10 +780,12 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
         return mScrollable;
     }
 
-    int getCurrentCharOffset() {
-        synchronized(mWidthLock) {
-            return (int) Math.round(mCurrentPos / mCharWidth);
-        }
+    /**
+     * Map pixel position to digit offset.
+     * UI thread only.
+     */
+    int getCharOffset(int pos) {
+        return (int) Math.round(pos / mCharWidth);  // Lock not needed.
     }
 
     void clear() {
@@ -589,11 +794,19 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
         setText("");
     }
 
+    /**
+     * Refresh display.
+     * Only called in UI thread.
+     */
     void redisplay() {
-        int currentCharOffset = getCurrentCharOffset();
+        int currentCharOffset = getCharOffset(mCurrentPos);
         int maxChars = getMaxChars();
         int lastDisplayedOffset[] = new int[1];
-        String result = getFormattedResult(currentCharOffset, maxChars, lastDisplayedOffset, false);
+        String result = getFormattedResult(currentCharOffset, maxChars, lastDisplayedOffset,
+                mAppendExponent /* forcePrecision; preserve entire result */,
+                !mWholePartFits
+                &&  currentCharOffset == getCharOffset(mMinPos) /* forceSciNotation */,
+                mWholePartFits /* insertCommas */ );
         int expIndex = result.indexOf('E');
         result = KeyMaps.translateResult(result);
         if (expIndex > 0 && result.indexOf('.') == -1) {
@@ -614,7 +827,7 @@ public class CalculatorResult extends AlignedTextView implements MenuItem.OnMenu
         if (!mScrollable) return;
         if (mScroller.computeScrollOffset()) {
             mCurrentPos = mScroller.getCurrX();
-            if (mCurrentPos != mLastPos) {
+            if (getCharOffset(mCurrentPos) != getCharOffset(mLastPos)) {
                 mLastPos = mCurrentPos;
                 redisplay();
             }
diff --git a/src/com/android/calculator2/CalculatorText.java b/src/com/android/calculator2/CalculatorText.java
index 52006ea..73df9ed 100644
--- a/src/com/android/calculator2/CalculatorText.java
+++ b/src/com/android/calculator2/CalculatorText.java
@@ -182,13 +182,13 @@ public class CalculatorText extends AlignedTextView implements MenuItem.OnMenuIt
      */
     public void changeTextTo(CharSequence newText) {
         final CharSequence oldText = getText();
-        if (startsWith(newText, oldText)) {
-            final int newLen = newText.length();
-            final int oldLen = oldText.length();
-            if (newLen == oldLen + 1) {
+        final char separator = KeyMaps.translateResult(",").charAt(0);
+        final CharSequence added = StringUtils.getExtensionIgnoring(newText, oldText, separator);
+        if (added != null) {
+            if (added.length() == 1) {
                 // The algorithm for pronouncing a single character doesn't seem
                 // to respect our hints.  Don't give it the choice.
-                final char c = newText.charAt(oldLen);
+                final char c = added.charAt(0);
                 final int id = KeyMaps.keyForChar(c);
                 final String descr = KeyMaps.toDescriptiveString(getContext(), id);
                 if (descr != null) {
@@ -196,8 +196,8 @@ public class CalculatorText extends AlignedTextView implements MenuItem.OnMenuIt
                 } else {
                     announceForAccessibility(String.valueOf(c));
                 }
-            } else if (newLen > oldLen) {
-                announceForAccessibility(newText.subSequence(oldLen, newLen));
+            } else if (added.length() != 0) {
+                announceForAccessibility(added);
             }
         } else {
             announceForAccessibility(newText);
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
diff --git a/src/com/android/calculator2/KeyMaps.java b/src/com/android/calculator2/KeyMaps.java
index 6a12f80..e82f35d 100644
--- a/src/com/android/calculator2/KeyMaps.java
+++ b/src/com/android/calculator2/KeyMaps.java
@@ -485,6 +485,8 @@ public class KeyMaps {
             sOutputForResultChar.put(')', ")");
             sOutputForResultChar.put('l', "l");
             sOutputForResultChar.put('n', "n");
+            sOutputForResultChar.put(',',
+                    String.valueOf(DecimalFormatSymbols.getInstance().getGroupingSeparator()));
             sOutputForResultChar.put('\u221A', "\u221A"); // SQUARE ROOT
             sOutputForResultChar.put('\u03C0', "\u03C0"); // GREEK SMALL LETTER PI
             addButtonToOutputMap('-', R.id.op_sub);
diff --git a/src/com/android/calculator2/StringUtils.java b/src/com/android/calculator2/StringUtils.java
new file mode 100644
index 0000000..4eef0e7
--- /dev/null
+++ b/src/com/android/calculator2/StringUtils.java
@@ -0,0 +1,94 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.calculator2;
+
+/**
+ * Some helpful methods operating on strings.
+ */
+
+public class StringUtils {
+
+    /**
+     * Return a string with n copies of c.
+     */
+    public static String repeat(char c, int n) {
+        final StringBuilder result = new StringBuilder();
+        for (int i = 0; i < n; ++i) {
+            result.append(c);
+        }
+        return result.toString();
+    }
+
+    /**
+     * Return a copy of the supplied string with commas added every three digits.
+     * The substring indicated by the supplied range is assumed to contain only
+     * a whole number, with no decimal point.
+     * Inserting a digit separator every 3 digits appears to be
+     * at least somewhat acceptable, though not necessarily preferred, everywhere.
+     * The grouping separator in the result is NOT localized.
+     */
+    public static String addCommas(String s, int begin, int end) {
+        // Resist the temptation to use Java's NumberFormat, which converts to long or double
+        // and hence doesn't handle very large numbers.
+        StringBuilder result = new StringBuilder();
+        int current = begin;
+        while (current < end && (s.charAt(current) == '-' || s.charAt(current) == ' ')) {
+            ++current;
+        }
+        result.append(s, begin, current);
+        while (current < end) {
+            result.append(s.charAt(current));
+            ++current;
+            if ((end - current) % 3 == 0 && end != current) {
+                result.append(',');
+            }
+        }
+        return result.toString();
+    }
+
+    /**
+     * Ignoring all occurrences of c in both strings, check whether old is a prefix of new.
+     * If so, return the remaining subsequence of whole. If not, return null.
+     */
+    public static CharSequence getExtensionIgnoring(CharSequence whole, CharSequence prefix,
+            char c) {
+        int wIndex = 0;
+        int pIndex = 0;
+        final int wLen = whole.length();
+        final int pLen = prefix.length();
+        while (true) {
+            while (pIndex < pLen && prefix.charAt(pIndex) == c) {
+                ++pIndex;
+            }
+            while (wIndex < wLen && whole.charAt(wIndex) == c) {
+                ++wIndex;
+            }
+            if (pIndex == pLen) {
+                break;
+            }
+            if (wIndex == wLen || whole.charAt(wIndex) != prefix.charAt(pIndex) ) {
+                return null;
+            }
+            ++pIndex;
+            ++wIndex;
+        }
+        while (wIndex < wLen && whole.charAt(wIndex) == c) {
+            ++wIndex;
+        }
+        return whole.subSequence(wIndex, wLen);
+    }
+}
diff --git a/src/com/android/calculator2/UnifiedReal.java b/src/com/android/calculator2/UnifiedReal.java
index 351797c..d3bc947 100644
--- a/src/com/android/calculator2/UnifiedReal.java
+++ b/src/com/android/calculator2/UnifiedReal.java
@@ -398,7 +398,7 @@ public class UnifiedReal {
         String digits = intScaled.toString();
         int len = digits.length();
         if (len < n + 1) {
-            digits = BoundedRational.repeat('0', n + 1 - len) + digits;
+            digits = StringUtils.repeat('0', n + 1 - len) + digits;
             len = n + 1;
         }
         return (negative ? "-" : "") + digits.substring(0, len - n) + "."