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

Proposed by Riccardo Padovani
Status: Merged
Approved by: Giulio Collura
Approved revision: 79
Merged at revision: 82
Proposed branch: lp:~rpadovani/ubuntu-calculator-app/closeBrackets150127
Merge into: lp:ubuntu-calculator-app
Diff against target: 25 lines (+12/-0)
1 file modified
app/ubuntu-calculator-app.qml (+12/-0)
To merge this branch: bzr merge lp:~rpadovani/ubuntu-calculator-app/closeBrackets150127
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Nicholas Skaggs (community) Needs Fixing
Bartosz Kosiorek Approve
Review via email: mp+247690@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Bartosz Kosiorek (gang65) wrote :

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
Revision history for this message
Riccardo Padovani (rpadovani) wrote :

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 :-)

Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

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

Revision history for this message
Giulio Collura (gcollura) wrote :

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!

Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Thanks.
Let's merge then.

review: Approve
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Re-merge with trunk; there are conflicts.

review: Needs Fixing
79. By Riccardo Padovani on 2015-01-29

Merged from trunk

Revision history for this message
Riccardo Padovani (rpadovani) wrote :

Merged

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/ubuntu-calculator-app.qml'
2--- app/ubuntu-calculator-app.qml 2015-01-27 00:41:25 +0000
3+++ app/ubuntu-calculator-app.qml 2015-01-29 22:00:52 +0000
4@@ -181,9 +181,21 @@
5 return;
6 }
7
8+ // We try to balance brackets to avoid mathJs errors
9+ var numberOfOpenedBrackets = (longFormula.match(/\(/g) || []).length -
10+ (longFormula.match(/\)/g) || []).length;
11+
12+ for (var i = 0; i < numberOfOpenedBrackets; i++) {
13+ formulaPush(')');
14+ }
15+
16 try {
17 var result = mathJs.eval(longFormula);
18 } catch(exception) {
19+ // If the formula isn't right and we added brackets, we remove them
20+ for (var i = 0; i < numberOfOpenedBrackets; i++) {
21+ deleteLastFormulaElement();
22+ }
23 console.log("Error: math.js " + exception.toString() + " engine formula:" + longFormula);
24 return false;
25 }

Subscribers

People subscribed via source and target branches