Rework the key insertion logic
Bug: 21495243
This changes the behavior to be much more compatible with L.
We generally do not allow consecutive binary operators to be inserted.
In the case of a minus operator however, the logic is more complicated.
We do allow a minus after multiplication, division and power.
When the minus is explicitly entered, so the user sees our corrections,
we do not allow a minus after additive operators. In pasted text,
we do.
We no longer reject additions that would result in implicit
multiplications. We do immediately reject binary operators in leading
contexts, e.g. after a left parenthesis, in which they must result in
a syntax error.
Change-Id: I1d35d74335371f6f113808d68a4f293b699d9bd0
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 @@
mCurrentButton = mEqualButton;
onEquals();
} else {
- addChars(String.valueOf(c));
+ addChars(String.valueOf(c), true);
redisplayAfterFormulaChange();
}
}
@@ -441,6 +438,7 @@
// 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 @@
}
}
+ /**
+ * 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 @@
}
break;
default:
- addKeyToExpr(id);
+ addExplicitKeyToExpr(id);
redisplayAfterFormulaChange();
break;
}
@@ -858,11 +868,15 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
}
+ /**
+ * 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 @@
}
/**
+ * 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) {