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/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.