am 017de989: Rework the key insertion logic

* commit '017de9893efd33e179db10bc71189e150bc3486d':
  Rework the key insertion logic

3 files changed, 110 insertions, 29 deletions

diff --git a/src/com/android/calculator2/Calculator.java b/src/com/android/calculator2/Calculator.java
index 87e011a..e0c8a93 100644
--- a/src/com/android/calculator2/Calculator.java
+++ b/src/com/android/calculator2/Calculator.java
@@ -14,9 +14,6 @@
  * limitations under the License.
  */
 
-// FIXME: Menu handling, particularly for cut/paste, is very ugly
-//        and not the way it was intended.
-//        Other menus are not handled brilliantly either.
 // TODO: Better indication of when the result is known to be exact.
 // TODO: Check and possibly fix accessability issues.
 // TODO: Copy & more general paste in formula?  Note that this requires
@@ -170,7 +167,7 @@ public class Calculator extends Activity
                         mCurrentButton = mEqualButton;
                         onEquals();
                     } else {
-                        addChars(String.valueOf(c));
+                        addChars(String.valueOf(c), true);
                         redisplayAfterFormulaChange();
                     }
             }
@@ -441,6 +438,7 @@ public class Calculator extends Activity
     // Add the given button id to input expression.
     // If appropriate, clear the expression before doing so.
     private void addKeyToExpr(int id) {
+        // FIXME: Other states?
         if (mCurrentState == CalculatorState.ERROR) {
             setState(CalculatorState.INPUT);
         } else if (mCurrentState == CalculatorState.RESULT) {
@@ -456,6 +454,18 @@ public class Calculator extends Activity
         }
     }
 
+    /**
+     * Add the given button id to input expression, assuming it was explicitly
+     * typed/touched.
+     * We perform slightly more aggressive correction than in pasted expressions.
+     */
+    private void addExplicitKeyToExpr(int id) {
+        if (mCurrentState == CalculatorState.INPUT && id == R.id.op_sub) {
+            mEvaluator.getExpr().removeTrailingAdditiveOperators();
+        }
+        addKeyToExpr(id);
+    }
+
     private void redisplayAfterFormulaChange() {
         // TODO: Could do this more incrementally.
         redisplayFormula();
@@ -514,7 +524,7 @@ public class Calculator extends Activity
                 }
                 break;
             default:
-                addKeyToExpr(id);
+                addExplicitKeyToExpr(id);
                 redisplayAfterFormulaChange();
                 break;
         }
@@ -858,11 +868,15 @@ public class Calculator extends Activity
         displayMessage(msg);
     }
 
-    // Add input characters to the end of the expression by mapping them to
-    // the appropriate button pushes when possible.  Leftover characters
-    // are added to mUnprocessedChars, which is presumed to immediately
-    // precede the newly added characters.
-    private void addChars(String moreChars) {
+    /**
+     * Add input characters to the end of the expression.
+     * Map them to the appropriate button pushes when possible.  Leftover characters
+     * are added to mUnprocessedChars, which is presumed to immediately precede the newly
+     * added characters.
+     * @param moreChars Characters to be added.
+     * @param explicit These characters were explicitly typed by the user.
+     */
+    private void addChars(String moreChars, boolean explicit) {
         if (mUnprocessedChars != null) {
             moreChars = mUnprocessedChars + moreChars;
         }
@@ -873,7 +887,11 @@ public class Calculator extends Activity
             int k = KeyMaps.keyForChar(c);
             if (k != View.NO_ID) {
                 mCurrentButton = findViewById(k);
-                addKeyToExpr(k);
+                if (explicit) {
+                    addExplicitKeyToExpr(k);
+                } else {
+                    addKeyToExpr(k);
+                }
                 if (Character.isSurrogate(c)) {
                     current += 2;
                 } else {
@@ -884,7 +902,11 @@ public class Calculator extends Activity
             int f = KeyMaps.funForString(moreChars, current);
             if (f != View.NO_ID) {
                 mCurrentButton = findViewById(f);
-                addKeyToExpr(f);
+                if (explicit) {
+                    addExplicitKeyToExpr(f);
+                } else {
+                    addKeyToExpr(f);
+                }
                 if (f == R.id.op_sqrt) {
                     // Square root entered as function; don't lose the parenthesis.
                     addKeyToExpr(R.id.lparen);
@@ -920,7 +942,7 @@ public class Calculator extends Activity
             mEvaluator.addSaved();
             redisplayAfterFormulaChange();
         } else {
-            addChars(item.coerceToText(this).toString());
+            addChars(item.coerceToText(this).toString(), false);
         }
         return true;
     }
diff --git a/src/com/android/calculator2/CalculatorExpr.java b/src/com/android/calculator2/CalculatorExpr.java
index 8b3cb40..73e09bf 100644
--- a/src/com/android/calculator2/CalculatorExpr.java
+++ b/src/com/android/calculator2/CalculatorExpr.java
@@ -359,35 +359,44 @@ class CalculatorExpr {
         return (KeyMaps.isBinary(o.mId));
     }
 
-    // Append press of button with given id to expression.
-    // Returns false and does nothing if this would clearly
-    // result in a syntax error.
+    /**
+     * Append press of button with given id to expression.
+     * If the insertion would clearly result in a syntax error, either just return false
+     * and do nothing, or make an adjustment to avoid the problem.  We do the latter only
+     * for unambiguous consecutive binary operators, in which case we delete the first
+     * operator.
+     */
     boolean add(int id) {
         int s = mExpr.size();
         int d = KeyMaps.digVal(id);
         boolean binary = KeyMaps.isBinary(id);
-        if (s == 0 && binary && id != R.id.op_sub) return false;
-        if (binary && hasTrailingBinary()
-            && (id != R.id.op_sub || isOperatorUnchecked(s-1, R.id.op_sub))) {
-            return false;
+        Token lastTok = s == 0 ? null : mExpr.get(s-1);
+        int lastOp = lastTok instanceof Operator ? ((Operator) lastTok).mId : 0;
+        // Quietly replace a trailing binary operator with another one, unless the second
+        // operator is minus, in which case we just allow it as a unary minus.
+        if (binary && !KeyMaps.isPrefix(id)) {
+            if (s == 0 || lastOp == R.id.lparen || KeyMaps.isFunc(lastOp)
+                    || KeyMaps.isPrefix(lastOp) && lastOp != R.id.op_sub) {
+                return false;
+            }
+            while (hasTrailingBinary()) {
+                delete();
+            }
+            // s invalid and not used below.
         }
         boolean isConstPiece = (d != KeyMaps.NOT_DIGIT || id == R.id.dec_point);
         if (isConstPiece) {
+            // Since we treat juxtaposition as multiplication, a constant can appear anywhere.
             if (s == 0) {
                 mExpr.add(new Constant());
                 s++;
             } else {
                 Token last = mExpr.get(s-1);
                 if(!(last instanceof Constant)) {
-                    if (!(last instanceof Operator)) {
-                        return false;
-                    }
-                    int lastOp = ((Operator)last).mId;
-                    if (lastOp == R.id.const_e || lastOp == R.id.const_pi
-                        || lastOp == R.id.op_fact
-                        || lastOp == R.id.rparen) {
-                        // Constant cannot possibly follow; reject immediately
-                        return false;
+                    if (last instanceof PreEval) {
+                        // Add explicit multiplication to avoid confusing display.
+                        mExpr.add(new Operator(R.id.op_mul));
+                        s++;
                     }
                     mExpr.add(new Constant());
                     s++;
@@ -400,6 +409,21 @@ class CalculatorExpr {
         }
     }
 
+    /**
+     * Remove trailing op_add and op_sub operators.
+     */
+    void removeTrailingAdditiveOperators() {
+        while (true) {
+            int s = mExpr.size();
+            if (s == 0) break;
+            Token lastTok = mExpr.get(s-1);
+            if (!(lastTok instanceof Operator)) break;
+            int lastOp = ((Operator) lastTok).mId;
+            if (lastOp != R.id.op_add && lastOp != R.id.op_sub) break;
+            delete();
+        }
+    }
+
     // Append the contents of the argument expression.
     // It is assumed that the argument expression will not change,
     // and thus its pieces can be reused directly.
diff --git a/src/com/android/calculator2/KeyMaps.java b/src/com/android/calculator2/KeyMaps.java
index 5385424..c28e80d 100644
--- a/src/com/android/calculator2/KeyMaps.java
+++ b/src/com/android/calculator2/KeyMaps.java
@@ -132,6 +132,41 @@ public class KeyMaps {
     }
 
     /**
+     * Does a button id correspond to a function that introduces an implicit lparen?
+     * Pure function.
+     */
+    public static boolean isFunc(int id) {
+        switch(id) {
+            case R.id.fun_sin:
+            case R.id.fun_cos:
+            case R.id.fun_tan:
+            case R.id.fun_arcsin:
+            case R.id.fun_arccos:
+            case R.id.fun_arctan:
+            case R.id.fun_ln:
+            case R.id.fun_log:
+            case R.id.fun_exp:
+                return true;
+            default:
+                return false;
+        }
+    }
+
+    /**
+     * Does a button id correspond to a prefix operator?
+     * Pure function.
+     */
+    public static boolean isPrefix(int id) {
+        switch(id) {
+            case R.id.op_sqrt:
+            case R.id.op_sub:
+                return true;
+            default:
+                return false;
+        }
+    }
+
+    /**
      * Does a button id correspond to a suffix operator?
      */
     public static boolean isSuffix(int id) {