Merge lp:~rpadovani/ubuntu-calculator-app/closeBrackets150127 into lp:ubuntu-calculator-app

[Riccardo Padovani]

New feature: auto close brackets at the end of a calc, if needed

New feature: auto close brackets at the end of a calc, if needed

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:78
http://91.189.93.70:8080/job/ubuntu-calculator-app-reboot-ci/72/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/837
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/837/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-reboot-vivid-amd64-ci/72

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calculator-app-reboot-ci/72/rebuild

review: Approve (continuous-integration)

[Bartosz Kosiorek (gang65)]

Thanks Riccardo
Frankly I would like to go some different way. I would like to do not modify user formula.
I don't want want to calculator be smarter than human, and modify your formula, without chance to correct it.

My idea (I wasn't have time to implement it yet) is just displaying small popup window, with error message, eg:
- "(10*10=" "Missing closing bracket"
- "20+20*=" "Operator at the end of formula is not allowed"
And for validation (it will not allow you to add object to formula):
- "1234.56." "It is not allowed to add two decimal separator in the same number"
- "234**" "It is not allowed to add two operator at once"
- "1+)" "Too many closing bracers"

I could create mockup with that.

My idea to have two strict rules:
1. Do not modify user formula "(20+20*=" "Operator at the end of formula is not allowed")
2. But, validate user formula during typing ("1234.56." "It is not allowed to add two decimal separator in the same number")

I'm open for discussion. Please let me know what you think about that approach.

review: Needs Information

[Riccardo Padovani (rpadovani)]

Hi Bartosz,

your idea is good, but I think it is for advanced users: a normal user IMO wants to do calc as fast as possible.

When I implemented this I was thinking to sqrt. I need to know the sqrt of a number (e.g 9). With this branch a user needs only 3 clicks:
- Open the app
- Click sqrt button
- Click 9
- Click =

The app then complete the calc with )

On the other side, with actual implementation (and with yours too) user needs to click 4 buttons and do two swipes:
- Open the app
- Click sqrt button
- Click 9
- (Maybe click = and see the popup)
- Swipe right
- Click )
- Swipe left
- Click (

So, both approaches have their advantages. I ping popey and gcollura to have feedbacks :-)

[Alan Pope 🍺🐧🐱 🦄 (popey)]

For what it's worth Gnome Calculator autocloses brackets.

6(6= returns 36.
I'd rather not have additional popups or hints, but the app to "do the right thing". Which, for brackets would be "assume the user is an idiot, and he meant to add a ')' here".

[Giulio Collura (gcollura)]

I totally support rpadovani's idea. Since this calculator app doesn't want to be a scientific calculator (btw, my scientific calc, the one I was during the exams, auto-closes brackets on short expressions), auto-closing brackets is a must-have feature.
While other kind of errors, like too many operators or too many brackets, must be marked as 'Syntax errors', maybe by highlighting with an animation the current calculation (without popover/popups that may annoy the user).
This is my point of view :)
You have my approval on this branch!

[Bartosz Kosiorek (gang65)]

Thanks.
Let's merge then.

review: Approve

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Autolanding.
More details in the following jenkins job:
http://91.189.93.70:8080/job/ubuntu-calculator-app-reboot-autolanding/46/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-vivid/877/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-calculator-app-reboot-vivid-amd64-autolanding/46/console

review: Needs Fixing (continuous-integration)

[Nicholas Skaggs (nskaggs)]

Re-merge with trunk; there are conflicts.

review: Needs Fixing

[lp:~rpadovani/ubuntu-calculator-app/closeBrackets150127]

79. By Riccardo Padovani on 2015-01-29

Merged from trunk

[Riccardo Padovani (rpadovani)]

Merged

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:79
http://91.189.93.70:8080/job/ubuntu-calculator-app-reboot-ci/80/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/884
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/884/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-reboot-vivid-amd64-ci/80

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calculator-app-reboot-ci/80/rebuild

review: Approve (continuous-integration)

=== modified file 'app/ubuntu-calculator-app.qml'
--- app/ubuntu-calculator-app.qml	2015-01-27 00:41:25 +0000
+++ app/ubuntu-calculator-app.qml	2015-01-29 22:00:52 +0000
@@ -181,9 +181,21 @@
             return;
         }
 
+        // We try to balance brackets to avoid mathJs errors
+        var numberOfOpenedBrackets = (longFormula.match(/\(/g) || []).length -
+                                        (longFormula.match(/\)/g) || []).length;
+
+        for (var i = 0; i < numberOfOpenedBrackets; i++) {
+            formulaPush(')');
+        }
+
         try {
             var result = mathJs.eval(longFormula);
         } catch(exception) {
+            // If the formula isn't right and we added brackets, we remove them
+            for (var i = 0; i < numberOfOpenedBrackets; i++) {
+                deleteLastFormulaElement();
+            }
             console.log("Error: math.js " + exception.toString() + " engine formula:" + longFormula);
             return false;
         }