After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 327590 - History View
History View
Status: RESOLVED FIXED
Product: gnome-calculator
Classification: Core
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: gcalctool maintainers
gcalctool maintainers
: 549319 676686 730933 (view as bug list)
Depends on:
Blocks: 681935
 
 
Reported: 2006-01-18 21:27 UTC by Kevin Duffus
Modified: 2014-12-22 21:25 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
Gzipped tar file of a history view for gcalctool. (3.78 KB, application/x-gzip)
2007-08-03 22:08 UTC, Rich Burridge
  Details
Reworked patch. (13.95 KB, patch)
2007-08-04 15:59 UTC, Rich Burridge
none Details | Review
Rereworked patch. (14.13 KB, patch)
2007-08-05 16:18 UTC, Rich Burridge
none Details | Review
A prototype of the future history panel that will be implemented (855.00 KB, application/zip)
2012-05-28 23:19 UTC, Gopal Krishnan
  Details
Screenshot of the prototype being used (11.40 KB, image/png)
2012-05-28 23:19 UTC, Gopal Krishnan
  Details
Creates a text view that sets out the start of the history view (2.26 KB, patch)
2012-06-14 23:11 UTC, Gopal Krishnan
none Details | Review
Changes the text view to a combo which is a much more convenient history view overall (1.59 KB, patch)
2012-06-14 23:13 UTC, Gopal Krishnan
none Details | Review
Appends the word test upon hitting the enter key (2.54 KB, patch)
2012-06-14 23:26 UTC, Gopal Krishnan
none Details | Review
Appends the equation to the combo box (1.05 KB, patch)
2012-06-14 23:27 UTC, Gopal Krishnan
none Details | Review
Displays the current equation as the active selection in the combo box (2.10 KB, patch)
2012-06-14 23:29 UTC, Gopal Krishnan
none Details | Review
Adds the lines that are important for the equals button to work (2.30 KB, patch)
2012-06-14 23:30 UTC, Gopal Krishnan
none Details | Review
The answer is appended to the combo box as well (3.71 KB, patch)
2012-06-14 23:34 UTC, Gopal Krishnan
none Details | Review
Fixes the display of the equation to the active text in the combo box (4.57 KB, patch)
2012-06-14 23:39 UTC, Gopal Krishnan
none Details | Review
Creates the History Panel (7.71 KB, patch)
2012-06-15 00:32 UTC, Gopal Krishnan
needs-work Details | Review
Creates the History Panel (6.55 KB, patch)
2012-06-26 15:11 UTC, Gopal Krishnan
none Details | Review
Creates a new module called Math-history.c (4.57 KB, patch)
2012-06-27 19:07 UTC, Gopal Krishnan
none Details | Review
Creates the history view's instance (1.80 KB, patch)
2012-06-27 19:09 UTC, Gopal Krishnan
none Details | Review
The history view no longer creates the GUI (12.93 KB, patch)
2012-06-27 19:11 UTC, Gopal Krishnan
none Details | Review
All the methods related to the history view are moved to the history module (7.81 KB, patch)
2012-06-27 19:14 UTC, Gopal Krishnan
none Details | Review
Changes the history view's combo box format to a tree view format (9.50 KB, patch)
2012-07-08 02:46 UTC, Gopal Krishnan
none Details | Review
Creates the history view completely (16.04 KB, patch)
2012-07-19 00:42 UTC, Gopal Krishnan
none Details | Review
Fixes the second row being cut off issue (2.01 KB, patch)
2012-07-19 17:31 UTC, Gopal Krishnan
none Details | Review
Allows the calculations to be appended on the equals button being clicked (4.11 KB, patch)
2012-07-19 18:39 UTC, Gopal Krishnan
none Details | Review
Sets the focus to the text view at the start (1.49 KB, patch)
2012-07-19 18:57 UTC, Gopal Krishnan
none Details | Review
Adds the missing signal files to the 2 patches before (3.87 KB, patch)
2012-07-19 19:06 UTC, Gopal Krishnan
none Details | Review
Renames certain parts of the code to the correct format (2.96 KB, patch)
2012-07-20 02:59 UTC, Gopal Krishnan
none Details | Review
Removed the mention of log in the signal name (1.94 KB, patch)
2012-07-23 16:07 UTC, Gopal Krishnan
none Details | Review
Creates the history view in three stages - text box, combo box and then tree view (21.53 KB, patch)
2012-08-01 18:33 UTC, Gopal Krishnan
needs-work Details | Review
Creates the history view (16.13 KB, patch)
2012-08-14 14:23 UTC, Gopal Krishnan
needs-work Details | Review
Fixes the whitespace and function name errors (5.48 KB, patch)
2012-08-17 20:24 UTC, Gopal Krishnan
none Details | Review
Elita's history view patch (13.24 KB, text/plain)
2014-06-04 22:54 UTC, Michael Catanzaro
  Details

Description Kevin Duffus 2006-01-18 21:27:34 UTC
I find it beneficial to have a view of previous calculations and solutions
similar to history view of Texas Instrument calculators. I came across a
calculator program for QT that does exactly what i was looking for (SpeedCrunch
- http://speedcrunch.berlios.de/). I would rather have this feature available in
gcalctool rather than having to install the QT libraries. Please consider adding
this feature. Thanks in advance.
Comment 1 Rich Burridge 2006-01-18 23:06:15 UTC
Yup. It's a good idea. I've even had it in the [g]calctool
for about 12 years. Someday...

Note that is should be fairly easy to add this in, in
arithmetic operator precedence mode, where the Return
nicely separates calculations. 
Comment 2 Martin Ammermüller 2007-01-29 21:01:52 UTC
I hope this feature will be implemented soon :) gcalctool is a really neat app, but what's really missing is a possibility to review and edit previous entries. E.g. you poked around with an equation on paper and entered it in gcalctool, get a result which doesn't make sense, review your equation and find out that a value should be in the denominator instead of the numerator of a fraction. Currently you have to enter the whole equation again, instead of recall it from a history and cut&paste that value in the right place, then let gcalctool recalculate it.

Just my 0.2€ :) 
Comment 3 Rich Burridge 2007-08-03 22:08:14 UTC
Created attachment 93063 [details]
Gzipped tar file of a history view for gcalctool.

Thanks to Afief Halumi for doing this. I'll take a look at it next week
and munge it into the same coding style as used throughout the rest of
gcalctool and add copyright headers etc to the two new files.

It's too late to go into gcalctool for GNOME 2.19/2.20 but it can be one of
the first things integrated into the next major release.
Comment 4 Rich Burridge 2007-08-04 15:59:56 UTC
Created attachment 93089 [details] [review]
Reworked patch.

Adjusted the original patch to do the following:

- Added new variables in their appropriate place (including alphabetic
  order).
- Removed tabs.  Tabs are evil.
- Added spaces where appropriate for better understanding of the code 
  or to be consistent with the rest of the code.
- Added braces ( {, } ) where needed.
- Adjusted code for 4 space indenting.
- Wrapped lines at 80 characters.
- Added _(...) around strings that will need to be localized.
- Removed variable names from function prototypes, so if they are
  changed at a later date, then they only have to be changed in one place.
- Added initial copyright headers to the two new files.
- Added the two new files to .../gcalctool/gcalctool/Makefile.am
- Added a ChangeLog entry.
- Fixed up compiler warnings generated by gcc.

I've left the debug messages in for now so it's easier to see 
what's going on.

It's possible that in transcribing your patch files by hand in 
order to adjust the code to the format I want, that I've 
accidentally broken things, because ...

Problems: 

1/ The gcalctool display area isn't being shown at all.
2/ Checking the "Show History" menu item doesn't show the history.

Question: if history_add() in history.c  always returns 0, why bother 
          returning anything?

From looking at speedcrunch (http://www.speedcrunch.org/), I suggest the
the History view be adjusted to a two column list (GtkTreeView widget).
The second column should be the results as you are doing now, but the
first column of each row should be the calculation that lead up to the
result. Both columns should be editable.

I also think it's possible that the history view will need to be disabled
(menu item made inactive and history view area hidden) if the user is in
Left-to-Right Precedence mode.

I'll work with Afief to come up with a patch that does what's needed.
Comment 5 Rich Burridge 2007-08-05 16:18:52 UTC
Created attachment 93107 [details] [review]
Rereworked patch.

I found the lines I was missing and have added them in.
This improves the patch a lot. At least in Arithmetic
Order Precedence mode.

If I try it in Left-To-Right Precedence mode and immediately
hit either the Up or the Down buttons, I see:

(gcalctool:26923): Gtk-CRITICAL **: gtk_text_buffer_set_text: assertion `text != NULL' failed

I think using a separate scrolling list with two columns, where
you can see all the entries and just use Cut and Paste to get them
will improve usability a lot. With no history visible, the Up/Down
buttons confused me a bit. Did I want to go up or down to get the
last entry?
Comment 6 Rich Burridge 2007-11-03 15:30:25 UTC
Now that we've got a "gcalctool-maint@gnome.bugs" alias, I'm
reassigning several bugs and enhancement back to that. They
are free to be picked up by one of the team and worked on.

Note that Afief Halumi has worked on this (thanks!) and there
is an attached patch to the bug. Now that gcalctool is Glade'd,
it should be much simpler to add in a History view.
Comment 7 Robert Ancell 2009-01-20 01:02:44 UTC
*** Bug 549319 has been marked as a duplicate of this bug. ***
Comment 8 Robert Ancell 2010-07-16 00:45:29 UTC
A partial solution might be to have the up and down arrows scroll through history like the terminal
Comment 9 Robert Ancell 2012-05-28 23:14:56 UTC
*** Bug 676686 has been marked as a duplicate of this bug. ***
Comment 10 Gopal Krishnan 2012-05-28 23:19:05 UTC
Created attachment 215155 [details]
A prototype of the future history panel that will be implemented

A prototype of the future history panel that will be implemented in my future patch
Comment 11 Gopal Krishnan 2012-05-28 23:19:45 UTC
Created attachment 215156 [details]
Screenshot of the prototype being used
Comment 12 Gopal Krishnan 2012-06-14 23:11:50 UTC
Created attachment 216477 [details] [review]
Creates a text view that sets out the start of the history view
Comment 13 Gopal Krishnan 2012-06-14 23:13:21 UTC
Created attachment 216478 [details] [review]
Changes the text view to a combo which is a much more convenient history view overall 

Changes the initial text view to a combo box which is much more useful for a history view.
Comment 14 Gopal Krishnan 2012-06-14 23:26:26 UTC
Created attachment 216480 [details] [review]
Appends the word test upon hitting the enter key

This will append test every time the enter key is hit. This is meant to replicate when an equation and answer will be appended to it.
Comment 15 Gopal Krishnan 2012-06-14 23:27:38 UTC
Created attachment 216481 [details] [review]
Appends the equation to the combo box

Instead of the word test now the equation entered by the user will be appended to the combo box.
Comment 16 Gopal Krishnan 2012-06-14 23:29:11 UTC
Created attachment 216482 [details] [review]
Displays the current equation as the active selection in the combo box

The current selection that is done by the user is set as the active text of the combo box
Comment 17 Gopal Krishnan 2012-06-14 23:30:39 UTC
Created attachment 216483 [details] [review]
Adds the lines that are important for the equals button to work

The lines required for the equals button on the calculator to append text to the combo box are entered. Its just a base work not the entire thing.
Comment 18 Gopal Krishnan 2012-06-14 23:34:37 UTC
Created attachment 216484 [details] [review]
The answer is appended to the combo box as well

The answer is appended to the combo box right after the equation is appended upon hitting the enter key.
Comment 19 Gopal Krishnan 2012-06-14 23:39:21 UTC
Created attachment 216485 [details] [review]
Fixes the display of the equation to the active text in the combo box

In the previous patch the setting of the current equation as the active text of the combo box was commented out. This uncomments it and redoes the layout of that particular portion for fixing the error in counting the exact position of the current equation.
Comment 20 Gopal Krishnan 2012-06-15 00:32:14 UTC
Created attachment 216488 [details] [review]
Creates the History Panel

This creates the history panel above the entry box and appends the equation and the answer it upon hitting the enter/return key.
Comment 21 Robert Ancell 2012-06-26 04:55:50 UTC
Review of attachment 216488 [details] [review]:

You're making progress but here's the main points to be taken in this review:
1. Make sure to match the style of the existing code
2. Check your patches for changes that aren't required or have no effect (e.g. whitespace)
3. Think carefully about where functionality should go.  A beginner mistake is to do calculations as early as possible after an event occurs, but that is rarely the correct place to do it.

Finally, please rewrite the commit comment to be one description of the patch instead of the concatenated comments from the commits that made up this patch.  You can do this by running git commit -a --amend.

Thanks Gopal!

::: src/math-display.c
@@ +35,3 @@
     GtkWidget *text_view;
 
+	/*History Panel*/

Make the indentation consistent with the existing code.  You've used a tab character here where the indentation is done with four spaces.

@@ +38,3 @@
+    GtkWidget *history_view;
+
+	/*Keeps track of number of equations*/

This should be /* Keeps track of number of equations */ (i.e. notice the whitespace).

@@ +39,3 @@
+
+	/*Keeps track of number of equations*/
+	gint counter;

Name is too generic.  It should be something like n_equations.

@@ +71,3 @@
     return display->priv->equation;
 }
+                                           

You've removed whitespace here for no reason.  Make sure that only intentional changes are in the patch (give it a quick look over before submitting, as it's easy to accidentally pick up changes like this).

@@ +78,3 @@
+	int ret;
+	char *equation;
+	MPEquationOptions options;

These variables are only used in the branch below - you can put them there to make the function more readable.

@@ +164,3 @@
+			gtk_combo_box_text_append(GTK_COMBO_BOX_TEXT(display->priv->history_view),0,result_str);
+			g_free(result_str);
+		}

This code shouldn't be done here - it should be done inside the MathEquation class.  The display should then listen for new results and update the history.

Think about it like this.  If at some point another part of the code triggers the equation to be solved then this code will never be run and the history will not be updated.

@@ +379,3 @@
     g_signal_connect(display, "key-press-event", G_CALLBACK(key_press_cb), display);
+	
+    display->priv->history_view = gtk_combo_box_text_new();

The history should be as shown in the design, i.e. the last n calculations.  You've used a combo-box which is not as useful as you can't glance at it and see your full calculation.

@@ +383,3 @@
+
+	//initialize the counter
+	//set to -1 so no if statement is required to check if its the first calculation or not

Use /* */ for comments

::: src/math-equation.c
@@ +26,3 @@
 #include "mp-enums.h"
 #include "unit-manager.h"
+#include "math-display.h"

All these changes do nothing.

::: src/math-window.c
@@ +389,3 @@
 static void
 create_gui(MathWindow *window)
+{	

Again, just a whitespace change.
Comment 22 Gopal Krishnan 2012-06-26 15:11:02 UTC
Created attachment 217303 [details] [review]
Creates the History Panel

Thanks Robert for the command! I used it and now this has a brief comment in regards to what I did in the history panel.

Gopal
Comment 23 Gopal Krishnan 2012-06-27 19:07:59 UTC
Created attachment 217450 [details] [review]
Creates a new module called Math-history.c

This contains the GUI creation as well as the methods needed to operate the history view
Comment 24 Gopal Krishnan 2012-06-27 19:09:40 UTC
Created attachment 217451 [details] [review]
Creates the history view's instance

This will actually create an instance of the history view.
Comment 25 Gopal Krishnan 2012-06-27 19:11:24 UTC
Created attachment 217452 [details] [review]
The history view no longer creates the GUI

The gui for the history view is no longer done by the history module. It is done instead by the math-display.c module
Comment 26 Gopal Krishnan 2012-06-27 19:14:26 UTC
Created attachment 217453 [details] [review]
All the methods related to the history view are moved to the history module

All the methods needed to modify the history view have been moved to the history module. In addition you can now select an equation/answer from the combo box and it will appear in the entry box.
Comment 27 Gopal Krishnan 2012-07-08 02:46:04 UTC
Created attachment 218244 [details] [review]
Changes the history view's combo box format to a tree view format

Currently the tree view is working perfectly except a few bugs/errors:

As it grows it flows behind the buttons thus making it unreadable beyond the first set of calculations. This was a better solution than placing it on top since that sends the entry box behind the buttons that is undesirable.

Besides this, upon selecting a value it is automatically inserted into the entry box. Both the equation and the answer are appended.
Comment 28 Gopal Krishnan 2012-07-19 00:42:13 UTC
Created attachment 219179 [details] [review]
Creates the history view completely

This is very close to the final release of the history view. This utilizes a tree view, the combo box code has been removed and in addition the whole pushing the entry box down upon appending has been fixed. Only problem that remains is the second row is cut off and in addition it does not work on the equals key but I am planning to remove that key from the UI  so that would depend on that.
Comment 29 Gopal Krishnan 2012-07-19 17:31:07 UTC
Created attachment 219249 [details] [review]
Fixes the second row being cut off issue

This will fix the second row being cut off issue and removes the last remanents of the combo box completely.
Comment 30 Gopal Krishnan 2012-07-19 18:39:12 UTC
Created attachment 219257 [details] [review]
Allows the calculations to be appended on the equals button being clicked

Finally when the equals button is clicked the equation and the answer are appended to the tree view and with this the history view is complete.
Comment 31 Gopal Krishnan 2012-07-19 18:57:42 UTC
Created attachment 219260 [details] [review]
Sets the focus to the text view at the start

Set the focus to the text view so typing can begin immediately either through keyboard or through the buttons on the UI.
Comment 32 Gopal Krishnan 2012-07-19 19:06:29 UTC
Created attachment 219261 [details] [review]
Adds the missing signal files to the 2 patches before

The previous 2 patches were missing out on the signal files and this adds that to make the history view work.
Comment 33 Gopal Krishnan 2012-07-20 02:59:42 UTC
Created attachment 219306 [details] [review]
Renames certain parts of the code to the correct format

Renames the signal and method names created by me to the customary format expected.
Comment 34 Gopal Krishnan 2012-07-23 16:07:35 UTC
Created attachment 219506 [details] [review]
Removed the mention of log in the signal name
Comment 35 Robert Ancell 2012-08-01 09:44:56 UTC
Gopal, all these open patches are for the same feature right? You need to merge them into one patch unless they are distinct features. In their current state they're not reviewable.
Comment 36 Gopal Krishnan 2012-08-01 12:10:52 UTC
Oh true there are nearly 20 patches so it would be a good idea to merge them. This is different from rebasing right? Additionally, I have to add another that comments this code because I have less comments right now. So I will finish that and then upload that one merged patch. Finally, are you going to push this finally?
Comment 37 Robert Ancell 2012-08-01 16:20:05 UTC
Rebasing is the same as merging. What I mean is there should only be one patch file per feature. Each patch will only be committed to master once it a) has been reviewed and b) improves the current codebase and c) doesn't cause any new problems. (This is standard procedure for any project).
Comment 38 Gopal Krishnan 2012-08-01 18:33:09 UTC
Created attachment 220092 [details] [review]
Creates the history view in three stages - text box, combo box and then tree view

I merged all the patches into one single patch. This creates the history view in three stages, whereby each stage is a new way to implement the exact same feature. Hence its in one patch as opposed to different patches. First the entry box followed by a combo box and then at last the tree view.
Comment 39 Robert Ancell 2012-08-14 06:33:22 UTC
Review of attachment 220092 [details] [review]:

You need to update the commit message to just describe this commit, e.g. "Add a history view". The commit message only needs additional information if you want to note the background to why this patch is required or any notable side-effects of it.

::: src/math-display.c
@@ +39,3 @@
 
+	/*History Panel*/
+	GtkWidget *list;

You have inconsistent whitespace, e.g. tab characters and space between /* and "History".

@@ +92,3 @@
+
+/* Upon selecting a value in the tree view it appears in the entry box as the current equation */
+static void  on_changed(GtkWidget *widget, MathDisplay *display) 

Note two spaces between the void and the on_changed and how this function is in a different style to init_list.

@@ +160,3 @@
     state = event->state & (GDK_CONTROL_MASK | GDK_MOD1_MASK);
     c = gdk_keyval_to_unicode(event->keyval);
+	

You've added whitespace here that changes nothing.

::: src/math-display.h
@@ +17,2 @@
 #include "math-equation.h"
+#include "math-history.h"

Why add this include? Nothing in the header file requires it.

::: src/math-history.c
@@ +48,3 @@
+    if (ret == PARSER_ERR_MP)
+    {
+        //nothing

"nothing" is not a useful comment - you should say something like "don't put errors into the history".

If you re-order this function it would read better, i.e.

/* Ignore errors */
if (ret == PARSER_ERR_MP || ret != 0)
    return;

...

::: src/math-window.c
@@ +251,3 @@
     gboolean result;
     g_signal_emit_by_name(window->priv->display, "key-press-event", event, &result);
+	//g_signal_emit_by_name(window->priv->history,"key-press-event",event,&result);

Why is this disabled? If it's no longer required then just remove the line.
Comment 40 Gopal Krishnan 2012-08-14 14:23:56 UTC
Created attachment 221143 [details] [review]
Creates the history view

Fixed the errors in the patch and edited the commit message as well.
Comment 41 Robert Ancell 2012-08-17 08:46:15 UTC
Review of attachment 221143 [details] [review]:

::: src/math-display.c
@@ +26,3 @@
     PROP_0,
+    PROP_EQUATION,
+  	LIST_ITEM = 0,

tab characters...

@@ +76,3 @@
+init_list(GtkWidget *list)
+{
+  GtkCellRenderer *renderer;

indentation is four spaces, not two.

@@ +164,3 @@
     /* Solve on enter */
     if (event->keyval == GDK_KEY_Return || event->keyval == GDK_KEY_KP_Enter) {
+	    math_equation_solve(display->priv->equation);

whitespace changes that do nothing

@@ +370,3 @@
+	MathDisplay *display = user_data;
+	/* Passes the equation to the tree view and then the answer */
+	add_to_list(display->priv->list,display->priv->equation,TRUE);

should be space after the comma

::: src/math-history.c
@@ +30,3 @@
+add_to_list(GtkWidget *list, MathEquation *equation, gboolean append_flag)
+{
+  	MpSerializer *result_serializer;

tab characters and whitespace are mixed here (make sure to view it in bugzilla to see what I am seeing)

::: src/math-history.h
@@ +1,1 @@
+/*

The math-history file should contain a MathHistory class that wraps all the details of the history.

@@ +21,3 @@
+G_BEGIN_DECLS
+
+void add_to_list(GtkWidget *list, MathEquation *equation, gboolean append_flag);

This should be named something like math_history_append, see another header like math-equation.h for naming conventions (consistent with other C GNOME programs).
Comment 42 Gopal Krishnan 2012-08-17 20:24:35 UTC
Created attachment 221669 [details] [review]
Fixes the whitespace and function name errors

Fixes all the remaining whitespace errors and the function name change that was required.
Comment 43 Michael Catanzaro 2014-05-29 14:28:57 UTC
Comment on attachment 221143 [details] [review]
Creates the history view

Le sigh
Comment 44 Michael Catanzaro 2014-05-29 14:30:05 UTC
*** Bug 730933 has been marked as a duplicate of this bug. ***
Comment 45 Michael Catanzaro 2014-06-04 22:54:17 UTC
Created attachment 277912 [details]
Elita's history view patch
Comment 47 Michael Catanzaro 2014-06-04 23:52:44 UTC
Review of attachment 277912 [details]:

Easy issues first:

* This patch was created manually, but we want patches to be created with 'git format-patch'.  Take a look at our contribution guidelines: https://wiki.gnome.org/Git/Developers
* The patch seems to work on both history view and the undo bug, which is a completely separate issue. These need to be split into two separate patches.

A few style issues:

* There are several whitespace errors (trailing whitespace) that should be fixed.  Try applying your patch with 'git apply' and it will warn you for each one.
* There needs to be a space placed before each opening parenthesis.
* There should not be a space placed before a closing parenthesis.
* There should be a space after, but not before, each comma.
* There should be a space before and after most other operators, such as =, ==, and &&.
* Braces go on lines of their own, and are not indented.
* Indentation is always four spaces. Yours is quite inconsistent.

You use several static variables and even a global variable. This is not correct: those should all be specific to a particular instance of MathDisplay.

Most importantly, the patch does not seem to implement the design for this feature. I see an extremely small window with a horizontal scrollbar above the normal input field. This isn't very usable; can you try to make it better match the design?

::: src/math-display.vala
@@ +23,3 @@
     public MathEquation equation { get { return _equation; } }
 
+    public static Gtk.ScrolledWindow swH;

What does swH stand for? Can you think of a more descriptive name for this variable?
Comment 48 PioneerAxon 2014-06-09 18:01:11 UTC
Review of attachment 277912 [details]:

Adding a line of comment before function or code segment, explaining what it does, is a good idea.

Plus what Michael said..

::: src/math-display.vala
@@ +65,3 @@
+        Gtk.TreeViewColumn secondcolumn = view.get_column(1);
+        secondcolumn.set_fixed_width(75);
+        secondcolumn.set_sizing(Gtk.TreeViewColumnSizing.FIXED);

Instead of assigning both columns fixed size, it'll be a good strategy to fix the answer column size (in percentage), and use grow-only mode for answer.

Even better design would be self adjusting width, which based on equations and answers, sets the width.

::: src/math-equation.vala
@@ +20,3 @@
+
+public static bool solve_flag;
+public static bool answer_flag;

I don't see the need of these global variables. These should be either part of MathEquationState (if I understand them correctly) or some other similar class.

@@ +24,2 @@
 /* Expression mode state */
+public class MathEquationState

Instead of making the class public, it's a good practice to keep them private and add methods to access the content.
Comment 49 PioneerAxon 2014-12-22 21:25:12 UTC
Kevin and all,

Thank you for the suggestion and discussion.

The feature has been implemented in 3.15.1.

Reference commit : https://git.gnome.org/browse/gnome-calculator/commit/?id=e5c307320db02e0f22b6e61e3b40109a1668a506

Hope this helps.. :)