Merge lp:~ubuntu-calculator-dev/ubuntu-calculator-app/ubuntu-calculator-app-remember-last-formula into lp:ubuntu-calculator-app
- ubuntu-calculator-app-remember-last-formula
- Merge into trunk
Status: | Needs review |
---|---|
Proposed branch: | lp:~ubuntu-calculator-dev/ubuntu-calculator-app/ubuntu-calculator-app-remember-last-formula |
Merge into: | lp:ubuntu-calculator-app |
Diff against target: |
233 lines (+50/-46) 2 files modified
app/ubuntu-calculator-app.qml (+49/-46) debian/changelog (+1/-0) |
To merge this branch: | bzr merge lp:~ubuntu-calculator-dev/ubuntu-calculator-app/ubuntu-calculator-app-remember-last-formula |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Bartosz Kosiorek | Needs Information | ||
Jenkins Bot | continuous-integration | Approve | |
Review via email: mp+278900@code.launchpad.net |
Commit message
Remember formula from InputField after application close (LP: #1520508)
Description of the change
Remember formula from InputField after application close (LP: #1520508)
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote : | # |
Riccardo Padovani (rpadovani) wrote : | # |
Hi Bartosz,
thanks for working on this.
Unfortunately, I don't think this is the right way to fix the problem: Qt.labs.settings writes on a file the setting, so you add a very big overhead: every time user presses a button there is an I/O operation, that is very expensive.
IMO the right direction to fix this bug is saving the actual formula only when app is closed (using Component.
Let me know what you think.
[0]: http://
Bartosz Kosiorek (gang65) wrote : | # |
From user point of view it will be great to save progress every change.
I will check the impact on performance and give you performance comparison.
Bartosz Kosiorek (gang65) wrote : | # |
I checked that and saving is very fast, so from user perspective there is more benefits of savings formula every change (due to out of memory, which happens when you will open too many tabs in webbrowser).
Could you please double check that issue with profiler?
Bartosz Kosiorek (gang65) wrote : | # |
I checked that and saving is very fast, so from user perspective there is more benefits of savings formula every change (due to out of memory, which happens when you will open too many tabs in webbrowser).
Could you please double check that issue with profiler?
Unmerged revisions
- 247. By Bartosz Kosiorek
-
Remember formula from InputField after application close (LP: #1520508)
Preview Diff
1 | === modified file 'app/ubuntu-calculator-app.qml' | |||
2 | --- app/ubuntu-calculator-app.qml 2015-11-28 20:44:02 +0000 | |||
3 | +++ app/ubuntu-calculator-app.qml 2015-11-28 23:18:30 +0000 | |||
4 | @@ -24,6 +24,7 @@ | |||
5 | 24 | import "engine" | 24 | import "engine" |
6 | 25 | import "engine/math.js" as MathJs | 25 | import "engine/math.js" as MathJs |
7 | 26 | import "engine/formula.js" as Formula | 26 | import "engine/formula.js" as Formula |
8 | 27 | import Qt.labs.settings 1.0 | ||
9 | 27 | 28 | ||
10 | 28 | MainView { | 29 | MainView { |
11 | 29 | id: mainView | 30 | id: mainView |
12 | @@ -40,14 +41,16 @@ | |||
13 | 40 | // This is our engine | 41 | // This is our engine |
14 | 41 | property var mathJs: MathJs.mathJs; | 42 | property var mathJs: MathJs.mathJs; |
15 | 42 | 43 | ||
24 | 43 | // Long form of formula, which are saved in the storage/history | 44 | property var settings: Settings { |
25 | 44 | property string longFormula: ""; | 45 | // Long form of formula, which are saved in the storage/history |
26 | 45 | 46 | property string longFormula: ""; | |
27 | 46 | // Engine's short form of formula. It is displayed in TextInput field | 47 | |
28 | 47 | property string shortFormula: ""; | 48 | // Engine's short form of formula. It is displayed in TextInput field |
29 | 48 | 49 | property string shortFormula: ""; | |
30 | 49 | // The formula converted to human eye, which will be displayed in text input field | 50 | |
31 | 50 | property string displayedInputText: ""; | 51 | // The formula converted to human eye, which will be displayed in text input field |
32 | 52 | property string displayedInputText: ""; | ||
33 | 53 | } | ||
34 | 51 | 54 | ||
35 | 52 | // If this is true we calculate a temporary result to show in the bottom label | 55 | // If this is true we calculate a temporary result to show in the bottom label |
36 | 53 | property bool isFormulaIsValidToCalculate: false; | 56 | property bool isFormulaIsValidToCalculate: false; |
37 | @@ -77,14 +80,14 @@ | |||
38 | 77 | function deleteLastFormulaElement() { | 80 | function deleteLastFormulaElement() { |
39 | 78 | isFormulaIsValidToCalculate = false; | 81 | isFormulaIsValidToCalculate = false; |
40 | 79 | if (textInputField.cursorPosition === textInputField.length) { | 82 | if (textInputField.cursorPosition === textInputField.length) { |
42 | 80 | longFormula = Formula.deleteLastFormulaElement(isLastCalculate, longFormula) | 83 | settings.longFormula = Formula.deleteLastFormulaElement(isLastCalculate, settings.longFormula) |
43 | 81 | } else { | 84 | } else { |
46 | 82 | var truncatedSubstring = Formula.deleteLastFormulaElement(isLastCalculate, longFormula.slice(0, textInputField.cursorPosition)) | 85 | var truncatedSubstring = Formula.deleteLastFormulaElement(isLastCalculate, settings.longFormula.slice(0, textInputField.cursorPosition)) |
47 | 83 | longFormula = truncatedSubstring + longFormula.slice(textInputField.cursorPosition, longFormula.length); | 86 | settings.longFormula = truncatedSubstring + settings.longFormula.slice(textInputField.cursorPosition, settings.longFormula.length); |
48 | 84 | } | 87 | } |
50 | 85 | shortFormula = longFormula; | 88 | settings.shortFormula = settings.longFormula; |
51 | 86 | 89 | ||
53 | 87 | displayedInputText = longFormula; | 90 | settings.displayedInputText = settings.longFormula; |
54 | 88 | if (truncatedSubstring) { | 91 | if (truncatedSubstring) { |
55 | 89 | textInputField.cursorPosition = truncatedSubstring.length; | 92 | textInputField.cursorPosition = truncatedSubstring.length; |
56 | 90 | } | 93 | } |
57 | @@ -95,9 +98,9 @@ | |||
58 | 95 | */ | 98 | */ |
59 | 96 | function clearFormula() { | 99 | function clearFormula() { |
60 | 97 | isFormulaIsValidToCalculate = false; | 100 | isFormulaIsValidToCalculate = false; |
64 | 98 | shortFormula = ""; | 101 | settings.shortFormula = ""; |
65 | 99 | longFormula = ""; | 102 | settings.longFormula = ""; |
66 | 100 | displayedInputText = ""; | 103 | settings.displayedInputText = ""; |
67 | 101 | } | 104 | } |
68 | 102 | 105 | ||
69 | 103 | /** | 106 | /** |
70 | @@ -126,28 +129,28 @@ | |||
71 | 126 | // formula, otherwise we continue with the old one | 129 | // formula, otherwise we continue with the old one |
72 | 127 | if ((!isNaN(visual) || (visual === ".")) && isLastCalculate) { | 130 | if ((!isNaN(visual) || (visual === ".")) && isLastCalculate) { |
73 | 128 | isFormulaIsValidToCalculate = false; | 131 | isFormulaIsValidToCalculate = false; |
75 | 129 | longFormula = displayedInputText = shortFormula = ""; | 132 | settings.longFormula = settings.displayedInputText = settings.shortFormula = ""; |
76 | 130 | } | 133 | } |
77 | 131 | // Add zero when decimal separator is not after number | 134 | // Add zero when decimal separator is not after number |
79 | 132 | if ((visual === ".") && ((isNaN(displayedInputText.slice(textInputField.cursorPosition - 1, textInputField.cursorPosition))) || (longFormula === ""))) { | 135 | if ((visual === ".") && ((isNaN(settings.displayedInputText.slice(textInputField.cursorPosition - 1, textInputField.cursorPosition))) || (settings.longFormula === ""))) { |
80 | 133 | visual = "0."; | 136 | visual = "0."; |
81 | 134 | } | 137 | } |
82 | 135 | isLastCalculate = false; | 138 | isLastCalculate = false; |
83 | 136 | 139 | ||
85 | 137 | // Validate whole longFormula if the cursor is at the end of string | 140 | // Validate whole settings.longFormula if the cursor is at the end of string |
86 | 138 | if (textInputField.cursorPosition === textInputField.length) { | 141 | if (textInputField.cursorPosition === textInputField.length) { |
87 | 139 | if (visual === "()") { | 142 | if (visual === "()") { |
89 | 140 | visual = Formula.determineBracketTypeToAdd(longFormula) | 143 | visual = Formula.determineBracketTypeToAdd(settings.longFormula) |
90 | 141 | } | 144 | } |
92 | 142 | if (Formula.validateStringForAddingToFormula(longFormula, visual) === false) { | 145 | if (Formula.validateStringForAddingToFormula(settings.longFormula, visual) === false) { |
93 | 143 | errorAnimation.restart(); | 146 | errorAnimation.restart(); |
94 | 144 | return; | 147 | return; |
95 | 145 | } | 148 | } |
96 | 146 | } else { | 149 | } else { |
97 | 147 | if (visual === "()") { | 150 | if (visual === "()") { |
99 | 148 | visual = Formula.determineBracketTypeToAdd(longFormula.slice(0, textInputField.cursorPosition)) | 151 | visual = Formula.determineBracketTypeToAdd(settings.longFormula.slice(0, textInputField.cursorPosition)) |
100 | 149 | } | 152 | } |
102 | 150 | if (Formula.validateStringForAddingToFormula(longFormula.slice(0, textInputField.cursorPosition), visual) === false) { | 153 | if (Formula.validateStringForAddingToFormula(settings.longFormula.slice(0, textInputField.cursorPosition), visual) === false) { |
103 | 151 | errorAnimation.restart(); | 154 | errorAnimation.restart(); |
104 | 152 | return; | 155 | return; |
105 | 153 | } | 156 | } |
106 | @@ -160,9 +163,9 @@ | |||
107 | 160 | // we display a temporary result instead the all operation | 163 | // we display a temporary result instead the all operation |
108 | 161 | if (isNaN(visual) && (visual.toString() !== ".") && isFormulaIsValidToCalculate) { | 164 | if (isNaN(visual) && (visual.toString() !== ".") && isFormulaIsValidToCalculate) { |
109 | 162 | try { | 165 | try { |
111 | 163 | shortFormula = formatBigNumber(mathJs.eval(shortFormula)); | 166 | settings.shortFormula = formatBigNumber(mathJs.eval(settings.shortFormula)); |
112 | 164 | } catch(exception) { | 167 | } catch(exception) { |
114 | 165 | console.log("Debug: Temp result: " + exception.toString() + " engine formula: " + shortFormula); | 168 | console.log("Debug: Temp result: " + exception.toString() + " engine formula: " + settings.shortFormula); |
115 | 166 | } | 169 | } |
116 | 167 | 170 | ||
117 | 168 | isFormulaIsValidToCalculate = false; | 171 | isFormulaIsValidToCalculate = false; |
118 | @@ -170,14 +173,14 @@ | |||
119 | 170 | 173 | ||
120 | 171 | // Adding the new operator to the formula | 174 | // Adding the new operator to the formula |
121 | 172 | if (textInputField.cursorPosition === textInputField.length ) { | 175 | if (textInputField.cursorPosition === textInputField.length ) { |
125 | 173 | longFormula += visual.toString(); | 176 | settings.longFormula += visual.toString(); |
126 | 174 | shortFormula += visual.toString(); | 177 | settings.shortFormula += visual.toString(); |
127 | 175 | displayedInputText = shortFormula; | 178 | settings.displayedInputText = settings.shortFormula; |
128 | 176 | } else { | 179 | } else { |
131 | 177 | longFormula = longFormula.slice(0, textInputField.cursorPosition) + visual.toString() + longFormula.slice(textInputField.cursorPosition, longFormula.length); | 180 | settings.longFormula = settings.longFormula.slice(0, textInputField.cursorPosition) + visual.toString() + settings.longFormula.slice(textInputField.cursorPosition, settings.longFormula.length); |
132 | 178 | shortFormula = longFormula; | 181 | settings.shortFormula = settings.longFormula; |
133 | 179 | var preservedCursorPosition = textInputField.cursorPosition; | 182 | var preservedCursorPosition = textInputField.cursorPosition; |
135 | 180 | displayedInputText = shortFormula; | 183 | settings.displayedInputText = settings.shortFormula; |
136 | 181 | textInputField.cursorPosition = preservedCursorPosition + visual.length; | 184 | textInputField.cursorPosition = preservedCursorPosition + visual.length; |
137 | 182 | } | 185 | } |
138 | 183 | 186 | ||
139 | @@ -191,21 +194,21 @@ | |||
140 | 191 | mathJs.config({ | 194 | mathJs.config({ |
141 | 192 | number: 'bignumber' | 195 | number: 'bignumber' |
142 | 193 | }); | 196 | }); |
144 | 194 | if ((longFormula === '') || (isLastCalculate === true)) { | 197 | if ((settings.longFormula === '') || (isLastCalculate === true)) { |
145 | 195 | errorAnimation.restart(); | 198 | errorAnimation.restart(); |
146 | 196 | return; | 199 | return; |
147 | 197 | } | 200 | } |
148 | 198 | 201 | ||
149 | 199 | // We try to balance brackets to avoid mathJs errors | 202 | // We try to balance brackets to avoid mathJs errors |
152 | 200 | var numberOfOpenedBrackets = (longFormula.match(/\(/g) || []).length - | 203 | var numberOfOpenedBrackets = (settings.longFormula.match(/\(/g) || []).length - |
153 | 201 | (longFormula.match(/\)/g) || []).length; | 204 | (settings.longFormula.match(/\)/g) || []).length; |
154 | 202 | 205 | ||
155 | 203 | for (var i = 0; i < numberOfOpenedBrackets; i++) { | 206 | for (var i = 0; i < numberOfOpenedBrackets; i++) { |
156 | 204 | formulaPush(')'); | 207 | formulaPush(')'); |
157 | 205 | } | 208 | } |
158 | 206 | 209 | ||
159 | 207 | try { | 210 | try { |
161 | 208 | var result = mathJs.eval(longFormula); | 211 | var result = mathJs.eval(settings.longFormula); |
162 | 209 | 212 | ||
163 | 210 | result = formatBigNumber(result) | 213 | result = formatBigNumber(result) |
164 | 211 | 214 | ||
165 | @@ -220,17 +223,17 @@ | |||
166 | 220 | 223 | ||
167 | 221 | isLastCalculate = true; | 224 | isLastCalculate = true; |
168 | 222 | 225 | ||
170 | 223 | if (result === longFormula) { | 226 | if (result === settings.longFormula) { |
171 | 224 | errorAnimation.restart(); | 227 | errorAnimation.restart(); |
172 | 225 | return; | 228 | return; |
173 | 226 | } | 229 | } |
174 | 227 | 230 | ||
176 | 228 | calculationHistory.addCalculationToScreen(longFormula, result, false, ""); | 231 | calculationHistory.addCalculationToScreen(settings.longFormula, result, false, ""); |
177 | 229 | editedCalculationIndex = -1; | 232 | editedCalculationIndex = -1; |
180 | 230 | longFormula = result; | 233 | settings.longFormula = result; |
181 | 231 | shortFormula = result; | 234 | settings.shortFormula = result; |
182 | 232 | favouriteTextField.text = ""; | 235 | favouriteTextField.text = ""; |
184 | 233 | displayedInputText = result; | 236 | settings.displayedInputText = result; |
185 | 234 | } | 237 | } |
186 | 235 | 238 | ||
187 | 236 | PageStack { | 239 | PageStack { |
188 | @@ -403,9 +406,9 @@ | |||
189 | 403 | iconName: "edit" | 406 | iconName: "edit" |
190 | 404 | text: i18n.tr("Edit") | 407 | text: i18n.tr("Edit") |
191 | 405 | onTriggered: { | 408 | onTriggered: { |
195 | 406 | longFormula = model.formula; | 409 | settings.longFormula = model.formula; |
196 | 407 | shortFormula = model.result; | 410 | settings.shortFormula = model.result; |
197 | 408 | displayedInputText = model.formula; | 411 | settings.displayedInputText = model.formula; |
198 | 409 | isLastCalculate = false; | 412 | isLastCalculate = false; |
199 | 410 | previousVisual = ""; | 413 | previousVisual = ""; |
200 | 411 | scrollableView.scrollToBottom(); | 414 | scrollableView.scrollToBottom(); |
201 | @@ -608,7 +611,7 @@ | |||
202 | 608 | } | 611 | } |
203 | 609 | } | 612 | } |
204 | 610 | 613 | ||
206 | 611 | text: Formula.returnFormulaToDisplay(displayedInputText) | 614 | text: Formula.returnFormulaToDisplay(settings.displayedInputText) |
207 | 612 | font.pixelSize: height * 0.7 | 615 | font.pixelSize: height * 0.7 |
208 | 613 | horizontalAlignment: TextInput.AlignRight | 616 | horizontalAlignment: TextInput.AlignRight |
209 | 614 | anchors { | 617 | anchors { |
210 | @@ -693,10 +696,10 @@ | |||
211 | 693 | if (cursorPosition !== length ) { | 696 | if (cursorPosition !== length ) { |
212 | 694 | // Count cursor position from the end of line | 697 | // Count cursor position from the end of line |
213 | 695 | var preservedCursorPosition = length - cursorPosition; | 698 | var preservedCursorPosition = length - cursorPosition; |
215 | 696 | displayedInputText = longFormula; | 699 | settings.displayedInputText = settings.longFormula; |
216 | 697 | cursorPosition = length - preservedCursorPosition; | 700 | cursorPosition = length - preservedCursorPosition; |
217 | 698 | } else { | 701 | } else { |
219 | 699 | displayedInputText = shortFormula; | 702 | settings.displayedInputText = settings.shortFormula; |
220 | 700 | } | 703 | } |
221 | 701 | 704 | ||
222 | 702 | SequentialAnimation { | 705 | SequentialAnimation { |
223 | 703 | 706 | ||
224 | === modified file 'debian/changelog' | |||
225 | --- debian/changelog 2015-11-28 20:44:02 +0000 | |||
226 | +++ debian/changelog 2015-11-28 23:18:30 +0000 | |||
227 | @@ -5,6 +5,7 @@ | |||
228 | 5 | * Upgrade math.js to version 2.4.2 to fix complex numbers formatting | 5 | * Upgrade math.js to version 2.4.2 to fix complex numbers formatting |
229 | 6 | * Run Calculator in Landscape mode for Desktop (LP: #1468663) | 6 | * Run Calculator in Landscape mode for Desktop (LP: #1468663) |
230 | 7 | * Add instruction how to enable profiling (workaround for LP: #1520551) | 7 | * Add instruction how to enable profiling (workaround for LP: #1520551) |
231 | 8 | * Remember formula from InputField after application close (LP: #1520508) | ||
232 | 8 | 9 | ||
233 | 9 | -- Bartosz Kosiorek <gang65@poczta.onet.pl> Thu, 12 Nov 2015 13:28:29 +0100 | 10 | -- Bartosz Kosiorek <gang65@poczta.onet.pl> Thu, 12 Nov 2015 13:28:29 +0100 |
234 | 10 | 11 |
PASSED: Continuous integration, rev:247 /core-apps- jenkins. ubuntu. com/job/ calculator- app-ci/ 35/ /core-apps- jenkins. ubuntu. com/job/ generic- update- mp/200/ console
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild: /core-apps- jenkins. ubuntu. com/job/ calculator- app-ci/ 35/rebuild
https:/