Merge lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-comma-without-number into lp:ubuntu-calculator-app

[Bartosz Kosiorek]

Allow add comma without number

Allow add comma without number

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

PASSED: Continuous integration, rev:191
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/332/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2946
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2946/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-utopic-amd64-ci/111
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-vivid-amd64-ci/54

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

review: Approve (continuous-integration)

[Niklas Wenzel (nikwen)]

Thanks. This is one of the features I always missed!

I have a few minor complaints though. ;)
There is an issue related to the removed code from couldAddDot() now. Typing in '4!.2' results in '4!0.2' while in my opinion it should be '4!*0.2'. The same applies to 'pi.2'.
Also try the following: Type in '23,4', place the cursor after the three and type in a plus. Shouldn't it insert a zero?

Check out my inline comment below as well. ;)

Thanks again!

review: Needs Fixing

[Bartosz Kosiorek (gang65)]

Please create new bug report regarding automatically adding "*" to formula.

With type '23,4': We could only parse/validate/modify formula before cursor, and in my opinion we shouldn't modify after. What if you would like to change something more in formula?
I would like to have full control of what I'm typing, and help user only when I'm 100% sure what is his intention.

[Alan Pope 🍺🐧🐱 🦄 (popey)]

Looks good to me.

review: Approve

[Riccardo Padovani (rpadovani)]

Doesn't work always.
Try to do 9+6*3+.3, 0 isn't added.

Please, also add a test case for this bug, and I agree with Niklas' inline comment, you shold add a comment to explain the code you added :-)

review: Needs Fixing

[lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-comma-without-number]

192. By Bartosz Kosiorek on 2015-06-21

Add comment to new code

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

PASSED: Continuous integration, rev:192
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/338/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2971
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2971/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-utopic-amd64-ci/117
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-vivid-amd64-ci/60

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

review: Approve (continuous-integration)

[Niklas Wenzel (nikwen)]

Bartosz, I filed the issue and thanks for adding the comment. ;)

[lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-comma-without-number]

193. By Bartosz Kosiorek on 2015-06-23

Fix issue with temporarly result

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

PASSED: Continuous integration, rev:193
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/341/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2999
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2999/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-utopic-amd64-ci/120
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-vivid-amd64-ci/63

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

review: Approve (continuous-integration)

[Niklas Wenzel (nikwen)]

Works really good now and it took like forever till I found an issue. ;)

1) Type in "8*8="
2) Type in "."

What happens:

The new input is "64.".

However, if we want to handle typing "." as "0.", we need to apply the same behaviour we apply to numbers here, i.e. clearing the input and inserting "0.".

Nevertheless, it works really well. Thanks. ;)

review: Needs Fixing

[lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-comma-without-number]

194. By Bartosz Kosiorek on 2015-07-01

Fixes after review

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

FAILED: Continuous integration, rev:194
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/349/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/3026/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-calculator-app-utopic-amd64-ci/128/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-calculator-app-vivid-amd64-ci/71/console

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

review: Needs Fixing (continuous-integration)

[Riccardo Padovani (rpadovani)]

There are conflicts now, do you mind to fix them? :-)

review: Needs Fixing

[lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-comma-without-number]

195. By Bartosz Kosiorek on 2015-07-04

Merge with trunk

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

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

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

review: Approve (continuous-integration)

=== modified file 'app/engine/formula.js'
--- app/engine/formula.js	2015-03-26 21:16:14 +0000
+++ app/engine/formula.js	2015-07-04 20:08:40 +0000
@@ -90,14 +90,22 @@
         return couldAddOperator(formula, stringToAddToFormula[0]);
     }
 
-    if (stringToAddToFormula === ".") {
+    // After decimal separator only number is allowed
+    if (isNaN(stringToAddToFormula)) {
+        if (formula.slice(-1) === ".") {
+            return false;
+        }
+    }
+
+    // We are ckecking last char, because we want to handle "0." string
+    if (stringToAddToFormula.slice(-1) === ".") {
         return couldAddDot(formula);
     }
 
     if (stringToAddToFormula === ")") {
         return couldAddCloseBracket(formula);
     }
-
+ 
     // Validate complex numbers
     if ((stringToAddToFormula === "i") || (!isNaN(stringToAddToFormula))){
         if (formula.slice(-1) === "i") {
@@ -226,11 +234,6 @@
  * @return bool: true if the dot could be added, false otherwhise
  */
 function couldAddDot(formulaToCheck) {
-    // A dot could be only after a number
-    if ((isNaN(formulaToCheck.slice(-1))) || (formulaToCheck === "")) {
-        return false;
-    }
-
     // If is after a number and it's the first dot of the calc it could be added
     if (formulaToCheck.indexOf('.') === -1) {
         return true;

=== modified file 'app/ubuntu-calculator-app.qml'
--- app/ubuntu-calculator-app.qml	2015-07-01 22:54:57 +0000
+++ app/ubuntu-calculator-app.qml	2015-07-04 20:08:40 +0000
@@ -136,10 +136,14 @@
         });
         // If the user press a number after the press of "=" we start a new
         // formula, otherwise we continue with the old one
-        if (!isNaN(visual) && isLastCalculate) {
+        if ((!isNaN(visual) || (visual === ".")) && isLastCalculate) {
             isFormulaIsValidToCalculate = false;
             longFormula = displayedInputText = shortFormula = "";
         }
+        // Add zero when decimal separator is not after number
+        if ((visual === ".") && ((isNaN(displayedInputText.slice(textInputField.cursorPosition - 1, textInputField.cursorPosition))) || (longFormula === ""))) {
+            visual = "0.";
+        }
         isLastCalculate = false;
 
         if (visual === "()") {
@@ -167,7 +171,7 @@
             try {
                 shortFormula = formatBigNumber(mathJs.eval(shortFormula));
             } catch(exception) {
-                console.log("Debug: Temp result: " + exception.toString() + " engine formula:" + shortFormula);
+                console.log("Debug: Temp result: " + exception.toString() + " engine formula: " + shortFormula);
             }
 
             isFormulaIsValidToCalculate = false;

=== modified file 'tests/autopilot/ubuntu_calculator_app/tests/test_main.py'
--- tests/autopilot/ubuntu_calculator_app/tests/test_main.py	2015-06-24 20:32:09 +0000
+++ tests/autopilot/ubuntu_calculator_app/tests/test_main.py	2015-07-04 20:08:40 +0000
@@ -199,6 +199,27 @@
         self.app.main_view.insert('-1=')
         self._assert_result_is(u'0.666666666667')
 
+    def test_adding_comma_without_number(self):
+        # Validation of the decimal separator
+        # We are trying to add several commas into one number
+        # Only first comma in the number should be allowed
+        self.app.main_view.insert('..1.3*.*5.=')
+        self._assert_result_is(u'0.065')
+        self._assert_history_contains(u'0.13×0.5=0.065')
+        self.app.main_view.insert('.7')
+        self._assert_result_is(u'0.7')
+
+    def test_adding_comma_without_number_on_temp_result(self):
+        self.app.main_view.insert('3+6*9')
+        self._assert_result_is(u'3+6×9')
+        self.app.main_view.insert('+')
+        self._assert_result_is(u'57+')
+        self.app.main_view.insert('.0')
+        self._assert_result_is(u'57+0.0')
+        self.app.main_view.insert('=')
+        self._assert_history_contains(u'3+6×9+0.0=57')
+        self._assert_result_is(u'57')
+
     def test_square(self):
         self.app.main_view.insert('4')
         self.app.main_view.show_scientific_keyboard()