-
Notifications
You must be signed in to change notification settings - Fork 17
Added calculator page #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| return x < 0 ? -x : x; | ||
| } | ||
|
|
||
| public void test_parse () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
| result_frame.add(result_box); | ||
|
|
||
| // Button grid | ||
| var grid = new Gtk.Grid(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
request: Can the calculator buttons fill to consume available space in the parent container?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
|
|
||
| public char get_keybinding() { | ||
| return 'o'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: why not use a?
Calculator
^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this seems much cleaner than using o
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A is used by the application page (default one), maybe i could use e for expression? or l ?
Calculator
^
or we could just move to a keychaining based approach where shortcuts can be two letter long with some threshold value between two consecutive keypresses to distiguish between like C and Ca to remove ambiguity
| namespace Ilia { | ||
| public class ExpressionParser : Object { | ||
|
|
||
| public static List<string> tokenize(string expr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
concern: this looks like a good start of a general purpose math evaluator but perhaps there is something available that has some maturity.. Perhaps it's possible to call into GCalc.Parser as a library? https://source.puri.sm/sebastian.krzyszkowiak/gnome-calculator/-/blob/pureos/byzantium/gcalc/gcalc-parser.vala#L59
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to find a library but they all seem too much for what i wanted to add, but thinking in retrospect we can always make use of more general calculator.
We should be able to use the libgcalc directly, i'll try to get that working
|
Moving forwrads i was thinking of
Outcomes Being able to evaluate all types of expresssions without any restrictions Being able to evavluate linear equations of form Evaluating simple conversions of form
In future we should also be able to include currency providers to do full list of features that should work https://github.com/GNOME/gnome-calculator/blob/main/tests/test-equation.vala would like to confirm before making this changes. This can also be easily archived by directly calling |
is it just this: https://github.com/GNOME/gnome-calculator/tree/main/gcalc? Anything else required (transitive dependencies, etc.) for it to work?
Tests are good, but if we leave the upstream source unchanged we would not expect to introduce a regression ourselves I'm thinking. Do you expect source changes would be needed in gcalc?
A direct integration is cleaner, agreed. Are there other advantages to this approach? Maybe we don't need to bundle upstream source but still get the features? |

Adding a simple calculator
Preview
Limitations
Currently it does not follow PEDMAS rule and strictly solves from left to write. Kept it this way to avoid complexity. could use something like libmatheval-dev in future to improve functionality.
Parenthesis do not work yet, expect for function calls like sin(exp), cos(exp)
Bindings
Since
Cwas already taken by Commands page, andKwas taken by Keybindings page, I decided to go withoas a shortcut key for calculator although i am not too happy with this and could just remove it altogether.