gnucash maint: Multiple changes pushed

Geert Janssens gjanssens at code.gnucash.org
Sun Sep 23 17:08:06 EDT 2018


Updated	 via  https://github.com/Gnucash/gnucash/commit/1c5eb86d (commit)
	 via  https://github.com/Gnucash/gnucash/commit/9bec660f (commit)
	 via  https://github.com/Gnucash/gnucash/commit/164f4847 (commit)
	from  https://github.com/Gnucash/gnucash/commit/3991ccb9 (commit)



commit 1c5eb86d9023d0505ea86d78a57030c7e3ef463e
Author: Geert Janssens <geert at kobaltwit.be>
Date:   Sun Sep 23 21:52:33 2018 +0200

    Simplify GNCQueryView's data model
    
    It was managing a number of redudant parameters which were leaking
    memory all over the place. The information that was tracked could
    easily be extracted from the underlying GtkTreeView, GtkTreeModel
    or GtkTreeSelection when needed.

diff --git a/gnucash/gnome-search/dialog-search.c b/gnucash/gnome-search/dialog-search.c
index eebd106..7db05bb 100644
--- a/gnucash/gnome-search/dialog-search.c
+++ b/gnucash/gnome-search/dialog-search.c
@@ -77,8 +77,6 @@ struct _GNCSearchWindow
 
     /* The "results" sub-window widgets */
     GtkWidget               *result_view;
-    gpointer                 selected_item;
-    GList                   *selected_item_list;
 
     /* The search_type radio-buttons */
     GtkWidget               *new_rb;
@@ -146,29 +144,24 @@ gnc_search_callback_button_execute (GNCSearchCallbackButton *cb,
                                     GNCSearchWindow *sw)
 {
     GNCQueryView     *qview = GNC_QUERY_VIEW(sw->result_view);
-    GtkTreeSelection *selection;
 
     // Sanity check
     g_assert(qview);
-    selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(qview));
-    g_assert(gtk_tree_selection_get_mode(selection) == GTK_SELECTION_MULTIPLE);
-    selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(qview));
 
     /* Do we have a callback for multi-selections ? */
     if (cb->cb_multiselect_fn && (!cb->cb_fcn ))
     {
-        /* We have allready populated the selected_item_list from the select row callback */
-        // We use g_list_prepend (for performance reasons), so we have to reverse once here
-        sw->selected_item_list = g_list_reverse(sw->selected_item_list);
-
+        GList *entries = gnc_query_view_get_selected_entry_list (qview);
         // Call the callback
-        (cb->cb_multiselect_fn)(GTK_WINDOW (sw->dialog), sw->selected_item_list, sw->user_data);
+        (cb->cb_multiselect_fn)(GTK_WINDOW (sw->dialog), entries, sw->user_data);
+        g_list_free (entries);
     }
     else
     {
         // No, stick to the single-item callback
+        gpointer entry = gnc_query_view_get_selected_entry (qview);
         if (cb->cb_fcn)
-            (cb->cb_fcn)(GTK_WINDOW (sw->dialog), &(sw->selected_item), sw->user_data);
+            (cb->cb_fcn)(GTK_WINDOW (sw->dialog), &entry, sw->user_data);
     }
 }
 
@@ -229,16 +222,18 @@ gnc_search_dialog_select_buttons_enable (GNCSearchWindow *sw, gint selected)
 static void
 gnc_search_dialog_select_cb (GtkButton *button, GNCSearchWindow *sw)
 {
+    gpointer entry;
     g_return_if_fail (sw->selected_cb);
 
-    if (sw->selected_item == NULL && sw->allow_clear == FALSE)
+    entry = gnc_query_view_get_selected_entry (GNC_QUERY_VIEW (sw->result_view));
+    if (!entry && !sw->allow_clear)
     {
         char *msg = _("You must select an item from the list");
         gnc_error_dialog (GTK_WINDOW (sw->dialog), "%s", msg);
         return;
     }
 
-    (sw->selected_cb)(GTK_WINDOW (sw->dialog), sw->selected_item, sw->select_arg);
+    (sw->selected_cb)(GTK_WINDOW (sw->dialog), entry, sw->select_arg);
     gnc_search_dialog_destroy (sw);
 }
 
@@ -249,22 +244,8 @@ gnc_search_dialog_select_row_cb (GNCQueryView *qview,
                                  gpointer user_data)
 {
     GNCSearchWindow  *sw = user_data;
-    gint              number_of_rows;
-
-    sw->selected_item_list = NULL;
-    sw->selected_item = NULL;
-
-    number_of_rows = GPOINTER_TO_INT(item);
-
+    gint number_of_rows = GPOINTER_TO_INT(item);
     gnc_search_dialog_select_buttons_enable(sw, number_of_rows);
-
-    if(number_of_rows == 1)
-    {
-        sw->selected_item = qview->selected_entry;
-        sw->selected_item_list = qview->selected_entry_list;
-    }
-    else
-        sw->selected_item_list = qview->selected_entry_list;
 }
 
 
@@ -275,7 +256,6 @@ gnc_search_dialog_double_click_cb (GNCQueryView *qview,
 {
     GNCSearchWindow  *sw = user_data;
 
-    sw->selected_item = item;
     if (sw->selected_cb)
         /* Select the item */
         gnc_search_dialog_select_cb (NULL, sw);
@@ -624,7 +604,15 @@ search_find_cb (GtkButton *button, GNCSearchWindow *sw)
     gnc_search_dialog_reset_widgets (sw);
 
     if (sw->result_cb)
-        (sw->result_cb)(sw->q, sw->user_data, &(sw->selected_item));
+    {
+        gpointer entry = NULL;
+        if (sw->result_view)
+        {
+            GNCQueryView *qview = GNC_QUERY_VIEW (sw->result_view);
+            entry = gnc_query_view_get_selected_entry (qview);
+        }
+        (sw->result_cb)(sw->q, sw->user_data, &entry);
+    }
     else
         gnc_search_dialog_display_results (sw);
 }
@@ -668,7 +656,6 @@ static void
 search_cancel_cb (GtkButton *button, GNCSearchWindow *sw)
 {
     /* Don't select anything */
-    sw->selected_item = NULL;
     gnc_search_dialog_destroy (sw);
 }
 
diff --git a/gnucash/gnome-utils/gnc-query-view.c b/gnucash/gnome-utils/gnc-query-view.c
index 78dbe3b..7ba8df8 100644
--- a/gnucash/gnome-utils/gnc-query-view.c
+++ b/gnucash/gnome-utils/gnc-query-view.c
@@ -32,6 +32,9 @@
 #include "gnc-query-view.h"
 #include "search-param.h"
 
+/* This static indicates the debugging module that this .o belongs to.  */
+static QofLogModule log_module = GNC_MOD_GUI;
+
 /* Signal codes */
 enum
 {
@@ -471,44 +474,11 @@ gnc_query_view_class_init (GNCQueryViewClass *klass)
 static void
 gnc_query_view_select_row_cb (GtkTreeSelection *selection, gpointer user_data)
 {
-    GNCQueryView   *qview = GNC_QUERY_VIEW (gtk_tree_selection_get_tree_view (selection));
-    GtkTreeModel   *model;
-    gint            number_of_rows;
-    gpointer        entry = NULL;
-    GList          *node;
-    GList          *list_of_rows;
-
-    qview->selected_entry_list = NULL;
-    qview->selected_entry = NULL;
-
-    model =  gtk_tree_view_get_model (GTK_TREE_VIEW (qview));
-    list_of_rows = gtk_tree_selection_get_selected_rows (selection, &model);
-    number_of_rows = gtk_tree_selection_count_selected_rows (selection);
-
-    /* We get a list of TreePaths */
-    for(node = list_of_rows; node; node = node->next)
-    {
-        GtkTreeIter iter;
-        if(gtk_tree_model_get_iter(model, &iter, node->data))
-        {
-            /* now iter is a valid row iterator */
-            gtk_tree_model_get (model, &iter, 0, &entry, -1);
-            if(number_of_rows == 1)
-            {
-                qview->selected_entry = entry;
-                qview->selected_entry_list = g_list_prepend(qview->selected_entry_list, entry);
-            }
-            else
-            {
-                qview->selected_entry = NULL;
-                qview->selected_entry_list = g_list_prepend(qview->selected_entry_list, entry);
-            }
-        }
-        gtk_tree_path_free(node->data);
-    }
-    g_list_free(list_of_rows);
+    GNCQueryView *qview = GNC_QUERY_VIEW (gtk_tree_selection_get_tree_view (selection));
+    gint number_of_rows = gtk_tree_selection_count_selected_rows (selection);
 
-    g_signal_emit (qview, query_view_signals[ROW_SELECTED], 0, GINT_TO_POINTER(number_of_rows));
+    g_signal_emit (qview, query_view_signals[ROW_SELECTED], 0,
+                   GINT_TO_POINTER(number_of_rows));
 }
 
 
@@ -528,9 +498,6 @@ gnc_query_view_double_click_cb (GtkTreeView       *view,
     if (gtk_tree_model_get_iter (model, &iter, path))
         gtk_tree_model_get (model, &iter, 0, &entry, -1);
 
-    qview->selected_entry = entry;
-    qview->selected_entry_list = NULL;
-
     g_signal_emit (qview, query_view_signals[DOUBLE_CLICK_ENTRY], 0, entry);
 }
 
@@ -563,14 +530,12 @@ gnc_query_view_toggled_cb (GtkCellRendererToggle *cell_renderer,
         indices = gtk_tree_path_get_indices (treepath);
         qview->toggled_row = indices[0];
         qview->toggled_column = column;
-        qview->selected_entry = entry;
 
         if(toggled)
             g_signal_emit (qview, query_view_signals[COLUMN_TOGGLED], 0, GINT_TO_POINTER(0));
         else
             g_signal_emit (qview, query_view_signals[COLUMN_TOGGLED], 0, GINT_TO_POINTER(1));
     }
-    qview->selected_entry = entry;
 }
 
 
@@ -586,12 +551,7 @@ gnc_query_view_destroy (GtkWidget *widget)
         gnc_unregister_gui_component (priv->component_id);
         priv->component_id = 0;
     }
-    /* Free the selected entry list */
-    if (qview->selected_entry_list)
-    {
-        g_list_free(qview->selected_entry_list);
-        qview->selected_entry_list = NULL;
-    }
+
     /* Remove the query */
     if (qview->query)
     {
@@ -606,30 +566,74 @@ gnc_query_view_destroy (GtkWidget *widget)
 gint
 gnc_query_view_get_num_entries (GNCQueryView *qview)
 {
+    GtkTreeModel *model;
+
     g_return_val_if_fail (qview != NULL, 0);
     g_return_val_if_fail (GNC_IS_QUERY_VIEW (qview), 0);
 
-    return qview->num_entries;
+    model = gtk_tree_view_get_model (GTK_TREE_VIEW (qview));
+    return gtk_tree_model_iter_n_children (model, NULL);
 }
 
 
 gpointer
 gnc_query_view_get_selected_entry (GNCQueryView *qview)
 {
+    gpointer entry = NULL;
+    GList *entries = NULL;
+    gint num_entries = 0;
+
     g_return_val_if_fail (qview != NULL, NULL);
     g_return_val_if_fail (GNC_IS_QUERY_VIEW (qview), NULL);
 
-    return qview->selected_entry;
+    entries = gnc_query_view_get_selected_entry_list (qview);
+    if (entries)
+        entry = entries->data;
+
+    num_entries = g_list_length (entries);
+    if (num_entries > 1)
+        PWARN ("Expected only one selected entry but found %i. "
+               "Discarding all but the first one.", num_entries);
+
+    g_list_free (entries);
+
+    return entry;
 }
 
+typedef struct
+{
+    GList *entries;
+} acc_data;
+
+static void
+accumulate_entries (GtkTreeModel *model, GtkTreePath *path,
+                    GtkTreeIter *iter, gpointer data)
+{
+    acc_data *acc_entries = (acc_data*)data;
+    gpointer entry = NULL;
+    GList *entries = acc_entries->entries;
+
+    gtk_tree_model_get (model, iter, 0, &entry, -1);
+    entries = g_list_prepend (entries, entry);
+    acc_entries->entries = entries;
+}
 
 GList *
 gnc_query_view_get_selected_entry_list (GNCQueryView *qview)
 {
+    GtkTreeSelection *selection;
+    acc_data acc_entries;
+    GList *entries = NULL;
+
     g_return_val_if_fail (qview != NULL, NULL);
     g_return_val_if_fail (GNC_IS_QUERY_VIEW (qview), NULL);
 
-    return qview->selected_entry_list;
+    acc_entries.entries = NULL;
+    selection = gtk_tree_view_get_selection (GTK_TREE_VIEW (qview));
+    gtk_tree_selection_selected_foreach (selection, accumulate_entries,
+                                         &acc_entries);
+    acc_entries.entries = g_list_reverse (acc_entries.entries);
+    return acc_entries.entries;
 }
 
 
@@ -685,24 +689,18 @@ void
 gnc_query_view_refresh (GNCQueryView *qview)
 {
     GtkTreeModel     *model;
-    GList            *old_entry;
+    GList            *selected_entries;
 
     g_return_if_fail (qview != NULL);
     g_return_if_fail (GNC_IS_QUERY_VIEW (qview));
 
-    old_entry = qview->selected_entry_list;
+    selected_entries = gnc_query_view_get_selected_entry_list (qview);
     model = gtk_tree_view_get_model (GTK_TREE_VIEW (qview));
     gtk_list_store_clear (GTK_LIST_STORE (model));
 
-    qview->num_entries = 0;
-    qview->selected_entry = NULL;
-    qview->selected_entry_list = NULL;
-
     gnc_query_view_fill (qview);
-
-    gnc_query_view_refresh_selected (qview, old_entry);
-
-    g_list_free(old_entry);
+    gnc_query_view_refresh_selected (qview, selected_entries);
+    g_list_free (selected_entries);
 }
 
 
@@ -850,8 +848,6 @@ gnc_query_view_fill (GNCQueryView *qview)
         guid = (const GncGUID*)((gup->param_getfcn)(item->data, gup));
         gnc_gui_component_watch_entity (priv->component_id, guid,
                                         QOF_EVENT_MODIFY | QOF_EVENT_DESTROY);
-
-        qview->num_entries++;
     }
 }
 
@@ -873,9 +869,6 @@ gnc_query_view_unselect_all (GNCQueryView *qview)
 
     selection = gtk_tree_view_get_selection (GTK_TREE_VIEW (qview));
     gtk_tree_selection_unselect_all (selection);
-
-    qview->selected_entry = NULL;
-    qview->selected_entry_list = NULL;
 }
 
 
diff --git a/gnucash/gnome-utils/gnc-query-view.h b/gnucash/gnome-utils/gnc-query-view.h
index 9c375cf..9798db0 100644
--- a/gnucash/gnome-utils/gnc-query-view.h
+++ b/gnucash/gnome-utils/gnc-query-view.h
@@ -47,11 +47,8 @@ extern "C" {
 
         /* Query information */
         Query      *query;
-        gint        num_entries;
 
         /* Select information */
-        gpointer    selected_entry;
-        GList      *selected_entry_list;
         gint        toggled_row;
         gint        toggled_column;
 
@@ -108,6 +105,8 @@ extern "C" {
 
     gpointer gnc_query_view_get_selected_entry (GNCQueryView *qview);
 
+    /** Returns a list of selected entries in the query view.
+     *  The returned GList should be freed by the caller */
     GList * gnc_query_view_get_selected_entry_list (GNCQueryView *qview);
 
     void gnc_query_view_refresh (GNCQueryView *qview);

commit 9bec660fba4ed92d86df05cf94501c6e3b7c1030
Author: Geert Janssens <geert at kobaltwit.be>
Date:   Sun Sep 23 18:14:35 2018 +0200

    Revert "Fix memory leaks in GncQueryView"
    
    This reverts commit 5609b704c569975663d3bf3b85ea50965f602399.
    
    While it fixed the memory leaks it also caused gnucash to crash
    when trying to search for invoices. Will be redone differently
    in a follow-up commit.

diff --git a/gnucash/gnome-utils/gnc-query-view.c b/gnucash/gnome-utils/gnc-query-view.c
index a77fed7..78dbe3b 100644
--- a/gnucash/gnome-utils/gnc-query-view.c
+++ b/gnucash/gnome-utils/gnc-query-view.c
@@ -219,8 +219,6 @@ gnc_query_view_init (GNCQueryView *qview)
     gnc_widget_set_style_context (GTK_WIDGET(qview), "GncQueryView");
 
     qview->query = NULL;
-    qview->selected_entry = NULL;
-    qview->selected_entry_list = NULL;
 
     qview->num_columns = 0;
     qview->column_params = NULL;
@@ -480,9 +478,8 @@ gnc_query_view_select_row_cb (GtkTreeSelection *selection, gpointer user_data)
     GList          *node;
     GList          *list_of_rows;
 
-    qview->selected_entry = NULL;
-    g_list_free (qview->selected_entry_list);
     qview->selected_entry_list = NULL;
+    qview->selected_entry = NULL;
 
     model =  gtk_tree_view_get_model (GTK_TREE_VIEW (qview));
     list_of_rows = gtk_tree_selection_get_selected_rows (selection, &model);
@@ -532,7 +529,6 @@ gnc_query_view_double_click_cb (GtkTreeView       *view,
         gtk_tree_model_get (model, &iter, 0, &entry, -1);
 
     qview->selected_entry = entry;
-    g_list_free (qview->selected_entry_list);
     qview->selected_entry_list = NULL;
 
     g_signal_emit (qview, query_view_signals[DOUBLE_CLICK_ENTRY], 0, entry);
@@ -652,7 +648,7 @@ gnc_query_view_refresh_selected (GNCQueryView *qview, GList *old_entry)
     model = gtk_tree_view_get_model (GTK_TREE_VIEW (qview));
     selection = gtk_tree_view_get_selection (GTK_TREE_VIEW (qview));
 
-    if(old_entry && g_list_length (old_entry) > 0)
+    if(g_list_length (old_entry) > 0)
     {
         /* Walk the list of old entries */
         for(node = old_entry; node; node = node->next)
@@ -688,23 +684,25 @@ gnc_query_view_refresh_selected (GNCQueryView *qview, GList *old_entry)
 void
 gnc_query_view_refresh (GNCQueryView *qview)
 {
-    GtkTreeModel *model;
+    GtkTreeModel     *model;
+    GList            *old_entry;
 
     g_return_if_fail (qview != NULL);
     g_return_if_fail (GNC_IS_QUERY_VIEW (qview));
 
+    old_entry = qview->selected_entry_list;
     model = gtk_tree_view_get_model (GTK_TREE_VIEW (qview));
     gtk_list_store_clear (GTK_LIST_STORE (model));
 
     qview->num_entries = 0;
     qview->selected_entry = NULL;
+    qview->selected_entry_list = NULL;
 
     gnc_query_view_fill (qview);
 
-    gnc_query_view_refresh_selected (qview, qview->selected_entry_list);
+    gnc_query_view_refresh_selected (qview, old_entry);
 
-    g_list_free(qview->selected_entry_list);
-    qview->selected_entry_list = NULL;
+    g_list_free(old_entry);
 }
 
 
@@ -877,7 +875,6 @@ gnc_query_view_unselect_all (GNCQueryView *qview)
     gtk_tree_selection_unselect_all (selection);
 
     qview->selected_entry = NULL;
-    g_list_free (qview->selected_entry_list);
     qview->selected_entry_list = NULL;
 }
 

commit 164f484718205d17499307cc804fc81dd574b607
Author: Geert Janssens <geert at kobaltwit.be>
Date:   Sun Sep 23 17:35:29 2018 +0200

    Open customer/vendor/employee report when double-clicking an the respective owner the cust/vend/empl overview page
    
    Before this action would open the cust/vend/empl edit window. However opening the report is a much
    more common use case so it makes sense to make that one default.

diff --git a/gnucash/gnome/gnc-plugin-page-owner-tree.c b/gnucash/gnome/gnc-plugin-page-owner-tree.c
index 389c779..4183842 100644
--- a/gnucash/gnome/gnc-plugin-page-owner-tree.c
+++ b/gnucash/gnome/gnc-plugin-page-owner-tree.c
@@ -895,13 +895,7 @@ gnc_plugin_page_owner_tree_double_click_cb (GtkTreeView        *treeview,
         GtkTreeViewColumn  *col,
         GncPluginPageOwnerTree *page)
 {
-    GncOwner *owner;
-    GtkWindow *parent;
-
-    g_return_if_fail (GNC_IS_PLUGIN_PAGE_OWNER_TREE (page));
-    owner = gnc_tree_view_owner_get_owner_from_path (GNC_TREE_VIEW_OWNER(treeview), path);
-    parent = GTK_WINDOW (gnc_plugin_page_get_window (GNC_PLUGIN_PAGE (page)));
-    gnc_ui_owner_edit (parent, owner);
+    gnc_plugin_page_owner_tree_cmd_owner_report (NULL, page);
 }
 
 static void



Summary of changes:
 gnucash/gnome-search/dialog-search.c       |  51 +++++------
 gnucash/gnome-utils/gnc-query-view.c       | 132 +++++++++++++----------------
 gnucash/gnome-utils/gnc-query-view.h       |   5 +-
 gnucash/gnome/gnc-plugin-page-owner-tree.c |   8 +-
 4 files changed, 83 insertions(+), 113 deletions(-)



More information about the gnucash-changes mailing list