Merge lp:~rpadovani/ubuntu-calculator-app/1179422 into lp:~ubuntu-calculator-dev/ubuntu-calculator-app/old_trunk

Proposed by Riccardo Padovani on 2013-08-20
Status: Merged
Approved by: Gustavo Pichorim Boiko on 2013-08-27
Approved revision: 148
Merged at revision: 141
Proposed branch: lp:~rpadovani/ubuntu-calculator-app/1179422
Merge into: lp:~ubuntu-calculator-dev/ubuntu-calculator-app/old_trunk
Diff against target: 208 lines (+77/-34)
3 files modified
formula.js (+62/-33)
tests/autopilot/ubuntu_calculator_app/emulators.py (+6/-1)
tests/autopilot/ubuntu_calculator_app/tests/test_simple_page.py (+9/-0)
To merge this branch: bzr merge lp:~rpadovani/ubuntu-calculator-app/1179422
Reviewer Review Type Date Requested Status
Gustavo Pichorim Boiko (community) Approve on 2013-08-27
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2013-08-27
Riccardo Padovani Approve on 2013-08-20
Review via email: mp+181010@code.launchpad.net

Commit message

Updated formula.js to allow continue calculation and update all functions.
Fixed bug #1179422
Fixed bug #1214069

Partially based on work of f-riccardo87. Thanks!

Thanks also to boiko for his support!

Description of the change

Updated formula.js to allow continue calculation and update all functions.
Fixed bug #1179422
Fixed bug #1214069

Partially based on work of f-riccardo87. Thanks!

To post a comment you must log in.
Riccardo Padovani (rpadovani) wrote :

Know bug:
- When change sign to a result, old result is still visible

To-do:
- Add new autopilot test, update old one
- Rewrite changeSign function
- Fix bug #1210082

review: Needs Fixing
review: Approve
Mihir Soni (mihirsoni) wrote :

Looks good to me.

Great work Riccardo..

Lets wait for Jenkins to successfully pass this.

Gustavo Pichorim Boiko (boiko) wrote :

Riccardo, I ended up doing the emulators.py fix in the other branch (to get tests running), so it might generate a conflict on your MR, sorry for that.

51 + // After user press equal the operation we have to forget the math order and restart the operation from the result, but set formula = result is impossible, because we lost a lot of precious information

Can you just break this comment into two lines? it is really long (this will also make sure CI gets re-run for this MR).

review: Needs Fixing
143. By Riccardo Padovani on 2013-08-25

Modified a long comment

144. By Riccardo Padovani on 2013-08-25

Update to avoid conflict with tree

145. By Riccardo Padovani on 2013-08-25

Updated to last tree version

146. By Riccardo Padovani on 2013-08-25

Maked pep8 happy :)

147. By Riccardo Padovani on 2013-08-27

Updated to tree

148. By Riccardo Padovani on 2013-08-27

Merged lp:~boiko/ubuntu-calculator-app/fix_get_result_label
Fix the code to get the result label. It should return None when there is no result.
Thanks to boiko!

Gustavo Pichorim Boiko (boiko) wrote :

Looks good and works as expected!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'formula.js'
2--- formula.js 2013-08-13 18:43:23 +0000
3+++ formula.js 2013-08-27 14:02:01 +0000
4@@ -23,12 +23,20 @@
5 function Formula() {
6 var previous = new Token(0, T_NUMBER);
7 var formula = '';
8- var results = [];
9 var lookupTbl = {
10 '-': '−',
11 '/': '÷',
12 '*': '×'
13 };
14+ var table = {
15+ "65" : "+",
16+ "66" : "-",
17+ "67" : "*",
18+ "68" : "/"
19+ }
20+
21+ var isLastResult = false; // This var becames true if the last button pressed by user is equal
22+ var isChangeSign = false; // This var becames true if user want to change the sign of a result
23
24 var _calculate = function(func){
25 try{
26@@ -48,25 +56,40 @@
27 }
28
29 this.push = function(digit){
30- if(results.length > 0) throw new SyntaxError("cannot push");
31 var last = (new Scanner(previous.value.toString()+digit, context)).tokens.pop();
32+
33+ if (!isNaN(digit) && isLastResult) return; // After a calc you have to add a sign before add a number
34+
35 switch(last.type){
36 case T_PLUS:
37 case T_MINUS:
38 case T_DIV:
39 case T_TIMES:
40- if(previous.type & T_OPERATOR) return;
41+ if (previous.type & T_OPERATOR) return; // Not two operator below
42 break;
43 case T_UNARY_PLUS:
44 case T_UNARY_MINUS:
45 if(previous.type === T_UNARY_MINUS || previous.type === T_UNARY_PLUS ) return;
46 break;
47 }
48+
49+ formulaView.headerItem.state = "newPush"; // This is to change "AC" to "C" when user continue a calc
50+
51+ /* After user press equal the operation we have to forget the math order
52+ * and restart the operation from the result,
53+ * but set formula = result is impossible,
54+ * because we lost a lot of precious information
55+ */
56+ if (isLastResult)
57+ formula = "(" + formula + ")";
58+
59 formula += digit;
60+ isLastResult = false;
61 previous = last;
62 var ret = {value: '', type: last.type};
63 //formula Shouldn't start with operators.
64- if(isNaN(formula.substring(0,1))){ // First letter should be digit
65+ var firstChar = formula.substring(0,1);
66+ if(isNaN(firstChar) && firstChar !== "(" && firstChar !== "-"){ // First letter should be digit or open bracket or minus
67 ret.type = T_OPERATOR // Return
68 formula = ""; // Make formula var empty
69 }
70@@ -79,10 +102,6 @@
71 };
72
73 this.pop = function(){
74- if(results.length > 0){
75- results.pop();
76- return;
77- }
78 var scannedFormula = new Scanner(formula, context);
79 var last = {type:T_NUMBER};
80 var removeSize = 0;
81@@ -99,42 +118,48 @@
82 var previousSize = previous.value.toString().length;
83 if(formula.length <= previousSize) return;
84 var scannedFormula = new Scanner(formula, context);
85+
86+ // Last and secondLast are one a number and the other an operator. Both can be both, it depends on user input
87 var last = scannedFormula.tokens.pop();
88- if(results.length > 0){
89- var newFormula = last.value.toString();
90- while(last.type !== T_PLUS &&
91- last.type !== T_MINUS &&
92- last.type !== T_DIV &&
93- last.type !== T_TIMES ){
94- last = scannedFormula.tokens.pop();
95- newFormula = last.value.toString() + newFormula;
96- }
97- result = _calculate(results[results.length-1]+newFormula);
98- } else{
99- // check previous input
100- if(last.type === T_PLUS ||
101- last.type === T_MINUS ||
102- last.type === T_DIV ||
103- last.type === T_TIMES){
104- // getting the first value
105- var firstVal = scannedFormula.tokens.pop();
106- // adding the first value to formula
107- formula = formula + firstVal.value.toString()
108- }
109- result = _calculate(formula);
110-
111+ var secondLast = scannedFormula.tokens.pop();
112+
113+ // In this case user press equal twice E.G. 2*2=4=8
114+ if (isLastResult && !isChangeSign && table[secondLast.type] !== undefined)
115+ formula = '(' + formula + ')' + table[secondLast.type] + last.value.toString();
116+ // In this case user press a number, and operator (or only an operator, if it isn't the first calc), and then equal
117+ else if (last.type & T_OPERATOR)
118+ formula = formula + secondLast.value.toString();
119+
120+ result = _calculate(formula);
121+
122+ // If user want to change sign to the result
123+ if (isLastResult && isChangeSign) {
124+ // TO-DO: check if the user has change the sign yet, and remove old *-1
125+ result = -result; // Change the sign
126+ formula = '(' + formula + ')*-1'; // Remember the change of the sign for the future
127 }
128- results.push(result.toString());
129+
130+ isLastResult = true;
131 return result;
132 }
133
134 this.changeSign = function(){
135- if(results.length > 0) throw new SyntaxError("cannot edit formula");
136+
137+ // If user want to change the sign of the result
138+ if (isLastResult) {
139+ isChangeSign = true;
140+ screenFormula.pop(); // Delete last result from the screen
141+ calculate();
142+ isChangeSign = false;
143+ return;
144+ }
145+
146 switch(previous.type){
147 case T_PLUS:
148 case T_MINUS:
149 case T_DIV:
150 case T_TIMES:
151+ // After a sign, push minus
152 formulaPush("-");
153 break;
154 default:
155@@ -142,6 +167,8 @@
156 var last = {type: undefined};
157 var toAdd = [];
158 var addMinus = true;
159+
160+ // Create a queue with all element to re-add to var formula
161 while(scannedFormula.tokens.length > 0 && last.type !== T_PLUS &&
162 last.type !== T_MINUS &&
163 last.type !== T_DIV &&
164@@ -154,7 +181,9 @@
165 addMinus = false;
166 }
167 }
168+
169 formulaPop();
170+
171 while(toAdd.length > 0){
172 var el = toAdd.pop()
173 formulaPush(el.value.toString());
174
175=== modified file 'tests/autopilot/ubuntu_calculator_app/emulators.py'
176--- tests/autopilot/ubuntu_calculator_app/emulators.py 2013-08-22 14:51:26 +0000
177+++ tests/autopilot/ubuntu_calculator_app/emulators.py 2013-08-27 14:02:01 +0000
178@@ -193,7 +193,12 @@
179 return result_label.numbers
180
181 def get_result_label(self):
182- return self.select_single('CalcLabel', objectName='result')
183+ results = self.select_many('CalcLabel', objectName='result')
184+ count = len(results)
185+ if count > 0:
186+ return results[count - 1]
187+ else:
188+ return None
189
190 def get_operand_by_index(self, index):
191 """Return an operand of a calculation entered."""
192
193=== modified file 'tests/autopilot/ubuntu_calculator_app/tests/test_simple_page.py'
194--- tests/autopilot/ubuntu_calculator_app/tests/test_simple_page.py 2013-08-23 17:48:47 +0000
195+++ tests/autopilot/ubuntu_calculator_app/tests/test_simple_page.py 2013-08-27 14:02:01 +0000
196@@ -216,3 +216,12 @@
197 # self.main_view.calculate_operation("*3")
198 #
199 # self._assert_result("")
200+
201+ def test_continue_calculation(self):
202+ self.main_view.calculate_operation("59+1")
203+
204+ calc_keyboard = self.main_view.get_calc_keyboard()
205+ calc_keyboard.enter_operation("+5")
206+ calc_keyboard.click_equals()
207+
208+ self._assert_result("65")

Subscribers

People subscribed via source and target branches