r17580 - gnucash/branches/2.2/src/gnome-utils - [r17441, r17470, r17471] Fix bugs in the commodity and price tree models.
Andreas Köhler
andi5 at cvs.gnucash.org
Sat Sep 20 22:50:27 EDT 2008
Author: andi5
Date: 2008-09-20 22:50:26 -0400 (Sat, 20 Sep 2008)
New Revision: 17580
Trac: http://svn.gnucash.org/trac/changeset/17580
Modified:
gnucash/branches/2.2/src/gnome-utils/gnc-tree-model-commodity.c
gnucash/branches/2.2/src/gnome-utils/gnc-tree-model-price.c
Log:
[r17441,r17470,r17471] Fix bugs in the commodity and price tree models.
Bug #376298, #539640: Fix bugs and clean up the Price Editor code.
-Fix row duplication associated with adding new rows (bug #376298)
-Fix row disappearance associated with editing commodities (bug #539640)
-Fix bugs in creating price iters in gnc_tree_model_price_iter_nth_child()
-Fix a bunch of inaccurate and/or crash-causing debugging statements
-Better address "race condition" (see code comments)
-Some limited whitespace cleanup
Update a few comments to fix typos and inaccuracies, or to add further
clarification.
Bug #454340: Prevent duplicate rows after editing a security. These changes are
similar to what was done for the price tree model in r17441 and r17470.
Committed by cedayiv.
Modified: gnucash/branches/2.2/src/gnome-utils/gnc-tree-model-commodity.c
===================================================================
--- gnucash/branches/2.2/src/gnome-utils/gnc-tree-model-commodity.c 2008-09-21 01:15:00 UTC (rev 17579)
+++ gnucash/branches/2.2/src/gnome-utils/gnc-tree-model-commodity.c 2008-09-21 02:50:26 UTC (rev 17580)
@@ -1,4 +1,4 @@
-/*
+/*
* gnc-tree-model-commodity.c -- GtkTreeModel implementation to
* display commodities in a GtkTreeView.
*
@@ -24,6 +24,11 @@
*/
/*
+ * In this model, valid paths take the form "X" or "X:Y", where:
+ * X is an index into the namespaces list held by the commodity db
+ * Y is an index into the commodity list for the namespace
+ *
+ * Iterators are populated with the following private data:
* iter->user_data Type NAMESPACE | COMMODITY
* iter->user_data2 A pointer to the namespace/commodity
* iter->user_data3 The index of the namespace/commodity within its parent list
@@ -289,10 +294,10 @@
/* Gnc Tree Model Debugging Utility Function */
/************************************************************/
-#define debug_path(fn, path) { \
- gchar *path_string = gtk_tree_path_to_string(path); \
- fn("tree path %s", path_string); \
- g_free(path_string); \
+#define debug_path(fn, path) { \
+ gchar *path_string = gtk_tree_path_to_string(path); \
+ fn("tree path %s", path_string? path_string : "NULL"); \
+ g_free(path_string); \
}
#define ITER_STRING_LEN 128
@@ -318,16 +323,16 @@
switch (GPOINTER_TO_INT(iter->user_data)) {
case GPOINTER_TO_INT(ITER_IS_NAMESPACE):
namespace = (gnc_commodity_namespace *) iter->user_data2;
- snprintf(string, ITER_STRING_LEN,
+ snprintf(string, ITER_STRING_LEN,
"[stamp:%x data:%d (NAMESPACE), %p (%s), %d]",
iter->stamp, GPOINTER_TO_INT(iter->user_data),
iter->user_data2, gnc_commodity_namespace_get_name (namespace),
GPOINTER_TO_INT(iter->user_data3));
break;
-
+
case GPOINTER_TO_INT(ITER_IS_COMMODITY):
commodity = (gnc_commodity *) iter->user_data2;
- snprintf(string, ITER_STRING_LEN,
+ snprintf(string, ITER_STRING_LEN,
"[stamp:%x data:%d (COMMODITY), %p (%s), %d]",
iter->stamp, GPOINTER_TO_INT(iter->user_data),
iter->user_data2, gnc_commodity_get_mnemonic (commodity),
@@ -335,7 +340,7 @@
break;
default:
- snprintf(string, ITER_STRING_LEN,
+ snprintf(string, ITER_STRING_LEN,
"[stamp:%x data:%d (UNKNOWN), %p, %d]",
iter->stamp,
GPOINTER_TO_INT(iter->user_data),
@@ -428,17 +433,10 @@
g_return_val_if_fail (path != NULL, FALSE);
depth = gtk_tree_path_get_depth (path);
- ENTER("model %p, iter, %p, path %p (depth %d)", tree_model, iter, path, depth);
+ ENTER("model %p, iter %p, path %p (depth %d)", tree_model, iter, path, depth);
debug_path(DEBUG, path);
- model = GNC_TREE_MODEL_COMMODITY (tree_model);
- priv = GNC_TREE_MODEL_COMMODITY_GET_PRIVATE(model);
- ct = priv->commodity_table;
- if (ct == NULL) {
- LEAVE("no commodity table");
- return FALSE;
- }
-
+ /* Check the path depth. */
if (depth == 0) {
LEAVE("depth too small");
return FALSE;
@@ -448,15 +446,26 @@
return FALSE;
}
+ /* Make sure the model has a commodity db. */
+ model = GNC_TREE_MODEL_COMMODITY (tree_model);
+ priv = GNC_TREE_MODEL_COMMODITY_GET_PRIVATE(model);
+ ct = priv->commodity_table;
+ if (ct == NULL) {
+ LEAVE("no commodity table");
+ return FALSE;
+ }
+
+ /* Verify the first part of the path: the namespace. */
list = gnc_commodity_table_get_namespaces_list(ct);
i = gtk_tree_path_get_indices (path)[0];
- {
- if (!(i >= 0 && i < g_list_length (list))) { LEAVE(""); }
- g_return_val_if_fail (i >= 0 && i < g_list_length (list), FALSE);
- }
namespace = g_list_nth_data (list, i);
+ if (!namespace) {
+ LEAVE("invalid path at namespace");
+ return FALSE;
+ }
if (depth == 1) {
+ /* Return an iterator for the namespace. */
iter->stamp = model->stamp;
iter->user_data = ITER_IS_NAMESPACE;
iter->user_data2 = namespace;
@@ -465,14 +474,16 @@
return TRUE;
}
+ /* Verify the second part of the path: the commodity. */
list = gnc_commodity_namespace_get_commodity_list(namespace);
i = gtk_tree_path_get_indices (path)[1];
- {
- if (!(i >= 0 && i < g_list_length (list))) { LEAVE(""); }
- g_return_val_if_fail (i >= 0 && i < g_list_length (list), FALSE);
- }
commodity = g_list_nth_data (list, i);
+ if (!commodity) {
+ LEAVE("invalid path at commodity");
+ return FALSE;
+ }
+ /* Return an iterator for the commodity. */
iter->stamp = model->stamp;
iter->user_data = ITER_IS_COMMODITY;
iter->user_data2 = commodity;
@@ -500,6 +511,7 @@
g_return_val_if_fail (iter->stamp == model->stamp, NULL);
ENTER("model %p, iter %p (%s)", tree_model, iter, iter_to_string(iter));
+ /* Make sure this model has a commodity db. */
priv = GNC_TREE_MODEL_COMMODITY_GET_PRIVATE(model);
ct = priv->commodity_table;
if (ct == NULL) {
@@ -508,14 +520,19 @@
}
if (iter->user_data == ITER_IS_NAMESPACE) {
+ /* Create a path to the namespace. This is just the index into
+ * the namespace list, which we already stored in user_data3. */
path = gtk_tree_path_new ();
gtk_tree_path_append_index (path, GPOINTER_TO_INT(iter->user_data3));
debug_path(LEAVE, path);
return path;
}
+ /* Get the namespaces list. */
ns_list = gnc_commodity_table_get_namespaces_list(ct);
namespace = gnc_commodity_get_namespace_ds((gnc_commodity*)iter->user_data2);
+
+ /* Create a path to the commodity. */
path = gtk_tree_path_new ();
gtk_tree_path_append_index (path, g_list_index (ns_list, namespace));
gtk_tree_path_append_index (path, GPOINTER_TO_INT(iter->user_data3));
@@ -1045,123 +1062,135 @@
static GSList *pending_removals = NULL;
-/** This function performs updating to the model after an commodity
- * has been added. The parent entry needs to be tapped on the
- * shoulder so that it can correctly update the disclosure triangle
- * (first added child) or possibly rebuild its child list of that
- * level of accounts is visible.
+/** This function updates the model when a row is being added. The
+ * immediate parent needs to be tapped on the shoulder so that it
+ * can correctly update the disclosure triangle (if this is its
+ * first child.)
*
* @internal
*
- * @param model The commodity tree model containing the commodity
- * that has been added.
+ * @param model The commodity tree model.
*
- * @param path The path to the newly added item.
+ * @param iter The iterator of the new row.
*/
static void
-gnc_tree_model_commodity_path_added (GncTreeModelCommodity *model,
- GtkTreeIter *iter)
+gnc_tree_model_commodity_row_add (GncTreeModelCommodity *model,
+ GtkTreeIter *iter)
{
GtkTreePath *path;
GtkTreeModel *tree_model;
GtkTreeIter tmp_iter;
ENTER("model %p, iter (%p)%s", model, iter, iter_to_string(iter));
+
+ /* We're adding a row, so the lists on which this model is based have
+ * changed. Since existing iterators (except the one just passed in)
+ * are all based on old indexes into those lists, we need to invalidate
+ * them, which we can do by changing the model's stamp. */
+ do {
+ model->stamp++;
+ } while (model->stamp == 0);
+ iter->stamp = model->stamp;
+
+ /* Tag the new row as inserted. */
tree_model = GTK_TREE_MODEL(model);
-
- /* For either namespace or commodity */
path = gnc_tree_model_commodity_get_path(tree_model, iter);
gtk_tree_model_row_inserted(tree_model, path, iter);
- /* */
- gtk_tree_path_up(path);
- while (gtk_tree_path_get_depth(path) != 0) {
- if (gtk_tree_model_get_iter(tree_model, &tmp_iter, path)) {
- gtk_tree_model_row_changed(tree_model, path, &tmp_iter);
- if (gtk_tree_model_iter_n_children(tree_model, &tmp_iter) == 1) {
- gtk_tree_model_row_has_child_toggled(tree_model, path, &tmp_iter);
- }
- }
- gtk_tree_path_up(path);
+ /* Inform all ancestors. */
+ /*
+ * Charles Day: I don't think calls to gtk_tree_model_row_changed() should
+ * be necessary. It is just a workaround for bug #540201.
+ */
+ if (gtk_tree_path_up(path) &&
+ gtk_tree_path_get_depth(path) > 0 &&
+ gtk_tree_model_get_iter(tree_model, &tmp_iter, path)) {
+ /* Signal the change to the parent. */
+ gtk_tree_model_row_changed(tree_model, path, &tmp_iter);
+
+ /* Is this the parent's first child? */
+ if (gtk_tree_model_iter_n_children(tree_model, &tmp_iter) == 1)
+ gtk_tree_model_row_has_child_toggled(tree_model, path, &tmp_iter);
+
+ /* Signal any other ancestors. */
+ while (gtk_tree_path_up(path) &&
+ gtk_tree_path_get_depth(path) > 0 &&
+ gtk_tree_model_get_iter(tree_model, &tmp_iter, path)) {
+ gtk_tree_model_row_changed(tree_model, path, &tmp_iter);
+ }
}
gtk_tree_path_free(path);
- do {
- model->stamp++;
- } while (model->stamp == 0);
+ /* If the new row already has children, signal that so the expander
+ * can be shown. This can happen, for example, if a namespace is
+ * changed in another place and gets removed and then re-added to
+ * the commodity db. */
+ if (gnc_tree_model_commodity_iter_has_child(tree_model, iter))
+ {
+ path = gnc_tree_model_commodity_get_path(tree_model, iter);
+ gtk_tree_model_row_has_child_toggled(tree_model, path, iter);
+ gtk_tree_path_free(path);
+ }
+
LEAVE(" ");
}
-
-/** This function performs common updating to the model after an
- * commodity has been removed. The parent entry needs to be tapped
- * on the shoulder so that it can correctly update the disclosure
- * triangle (last removed child) or possibly rebuild its child list
- * of that level of accounts is visible.
+/** This function updates the model when a row is being deleted.
+ * The immediate parent needs to be tapped on the shoulder so that it
+ * can correctly update the disclosure triangle (if this was its last
+ * child.)
*
* @internal
*
- * @param model The commodity tree model containing the commodity
- * that has been added or deleted.
+ * @param model The commodity model that is losing a row.
*
- * @param path The path to the newly added item, or the just removed
- * item.
+ * @param path The path of the row being removed.
*/
static void
-gnc_tree_model_commodity_path_deleted (GncTreeModelCommodity *model,
- GtkTreePath *path_in)
+gnc_tree_model_commodity_row_delete (GncTreeModelCommodity *model,
+ GtkTreePath *path)
{
- gnc_commodity_namespace *namespace;
- GtkTreePath *path;
+ GtkTreeModel *tree_model;
GtkTreeIter iter;
- GList *list;
- gint depth;
- gboolean del_row = TRUE;
- path = gtk_tree_path_copy(path_in);
+ g_return_if_fail(GNC_IS_TREE_MODEL_COMMODITY(model));
+ g_return_if_fail(path);
+
debug_path(ENTER, path);
- depth = gtk_tree_path_get_depth(path);
- if (depth == 2) {
- /* It seems sufficient to tell the model that the parent row
- * changed. This appears to force a reload of all its child rows,
- * which handles removing the now gone commodity. */
- if (gtk_tree_path_up (path)) {
- gnc_tree_model_commodity_get_iter (GTK_TREE_MODEL(model), &iter, path);
- debug_path(DEBUG, path);
- DEBUG("iter %s", iter_to_string(&iter));
- gtk_tree_model_row_changed (GTK_TREE_MODEL(model), path, &iter);
- namespace = gnc_tree_model_commodity_get_namespace (model, &iter);
- if (namespace) {
- list = gnc_commodity_namespace_get_commodity_list(namespace);
- if (g_list_length(list) == 0) {
- gtk_tree_model_row_has_child_toggled(GTK_TREE_MODEL(model), path, &iter);
- del_row = FALSE;
- }
- }
- }
- }
- gtk_tree_path_free(path);
+ tree_model = GTK_TREE_MODEL(model);
- /* Delete the row, if it hasn't already been deleted by an update of
- * the parent row. */
- if (del_row)
- gtk_tree_model_row_deleted (GTK_TREE_MODEL(model), path_in);
-
+ /* We're removing a row, so the lists on which this model is based have
+ * changed. Since existing iterators are all based on old indexes into
+ * those lists, we need to invalidate them, which we can do by changing
+ * the model's stamp. */
do {
model->stamp++;
} while (model->stamp == 0);
+
+ /* Signal that the path has been deleted. */
+ gtk_tree_model_row_deleted(tree_model, path);
+
+ /* Issue any appropriate signals to ancestors. */
+ if (gtk_tree_path_up(path) &&
+ gtk_tree_path_get_depth(path) > 0 &&
+ gtk_tree_model_get_iter(tree_model, &iter, path) &&
+ !gtk_tree_model_iter_has_child(tree_model, &iter)) {
+ DEBUG("parent toggled, iter %s", iter_to_string(&iter));
+ gtk_tree_model_row_has_child_toggled(tree_model, path, &iter);
+ }
+
LEAVE(" ");
}
/** This function is a one-shot helper routine for the following
- * gnc_tree_model_price_event_handler() function. It must be armed
+ * gnc_tree_model_commodity_event_handler() function. It must be armed
* each time an item is removed from the model. This function will
* be called as an idle function some time after the user requests
- * the deleteion. (Most likely when the event handler for the "OK"
- * button click returns. This function will send the "row_deleted"
- * signal to any/all parent models for each entry deleted.
+ * the deletion. (Most likely when the event handler for the "OK"
+ * button click returns.) This function will send the "row_deleted"
+ * signal to the model for each entry deleted.
*
* @internal
*
@@ -1174,48 +1203,60 @@
static gboolean
gnc_tree_model_commodity_do_deletions (gpointer unused)
{
- GSList *iter, *next = NULL;
- remove_data *data;
+ ENTER(" ");
- for (iter = pending_removals; iter != NULL; iter = next) {
- next = g_slist_next(iter);
- data = iter->data;
- pending_removals = g_slist_delete_link (pending_removals, iter);
+ /* Go through the list of paths needing removal. */
+ while (pending_removals)
+ {
+ remove_data *data = pending_removals->data;
+ pending_removals = g_slist_delete_link(pending_removals, pending_removals);
- gnc_tree_model_commodity_path_deleted (data->model, data->path);
- gtk_tree_path_free(data->path);
- g_free(data);
+ if (data)
+ {
+ debug_path(DEBUG, data->path);
+
+ /* Remove the path. */
+ gnc_tree_model_commodity_row_delete(data->model, data->path);
+
+ gtk_tree_path_free(data->path);
+ g_free(data);
+ }
}
- /* Remove me */
+ LEAVE(" ");
+ /* Don't call me again. */
return FALSE;
}
-/** This function is the handler for all event messages from the
- * engine. Its purpose is to update the commodity tree model any
- * time a commodity or namespace is added to the engine or deleted
- * from the engine. This change to the model is then propagated to
- * any/all overlying filters and views. This function listens to the
- * ADD, REMOVE, and DESTROY events.
+/** This function is the handler for all event messages from the engine.
+ * Its purpose is to update the tree model any time a commodity or
+ * namespace is added to the engine, modified, or deleted from the engine.
+ * This change to the model is then propagated to any/all overlying filters
+ * and views. This function listens to the ADD, REMOVE, MODIFY, and DESTROY
+ * events.
*
* @internal
*
- * @warning There is a "Catch 22" situation here.
- * gtk_tree_model_row_deleted() can't be called until after the item
- * has been deleted from the real model (which is the engine's
- * commodity table for us), but once the commodity has been deleted
+ * @warning There is a "Catch 22" situation here. The REMOVE event
+ * indicates that a namespace or commodity is *about* to be
+ * removed. But we can't actually delete the row by calling
+ * gtk_tree_model_row_deleted() until after the item has *actually*
+ * been removed from the real model (which is the engine's commodity
+ * table). But once the item has been deleted
* from the engine we have no way to determine the path to pass to
- * row_deleted(). This is a PITA, but the only ither choice is to
- * have this model mirror the engine's commodity table instead of
+ * row_deleted(). So we will save the path now, then call a function
+ * to delete the row when we receive the next event or we're idle
+ * (whichever comes first.) This is a PITA, but the only other choice
+ * is to have this model mirror the engine's commodity table instead of
* referencing it directly.
*
* @param entity The affected item.
*
* @param event type The type of the event. This function only cares
- * about items of type ADD, REMOVE, and DESTROY.
+ * about items of type ADD, REMOVE, MODIFY, and DESTROY.
*
- * @param user_data A pointer to the account tree model.
+ * @param user_data A pointer to the tree model.
*
* @param event_data A pointer to additional data about this event.
*/
@@ -1239,6 +1280,10 @@
ENTER("entity %p, event %d, model %p, event data %p",
entity, event_type, user_data, event_data);
+ /* Do deletions if any are pending. */
+ if (pending_removals)
+ gnc_tree_model_commodity_do_deletions(NULL);
+
/* get type specific data */
if (GNC_IS_COMMODITY(entity)) {
gnc_commodity *commodity;
@@ -1263,7 +1308,7 @@
}
}
} else {
- LEAVE("");
+ LEAVE("");
return;
}
@@ -1271,7 +1316,7 @@
case QOF_EVENT_ADD:
/* Tell the filters/views where the new account was added. */
DEBUG("add %s", name);
- gnc_tree_model_commodity_path_added (model, &iter);
+ gnc_tree_model_commodity_row_add (model, &iter);
break;
case QOF_EVENT_REMOVE:
@@ -1289,6 +1334,7 @@
pending_removals = g_slist_append (pending_removals, data);
g_idle_add_full(G_PRIORITY_HIGH_IDLE,
gnc_tree_model_commodity_do_deletions, NULL, NULL);
+
LEAVE(" ");
return;
Modified: gnucash/branches/2.2/src/gnome-utils/gnc-tree-model-price.c
===================================================================
--- gnucash/branches/2.2/src/gnome-utils/gnc-tree-model-price.c 2008-09-21 01:15:00 UTC (rev 17579)
+++ gnucash/branches/2.2/src/gnome-utils/gnc-tree-model-price.c 2008-09-21 02:50:26 UTC (rev 17580)
@@ -1,4 +1,4 @@
-/*
+/*
* gnc-tree-model-price.c -- GtkTreeModel implementation to display
* prices in a GtkTreeView.
*
@@ -24,9 +24,15 @@
*/
/*
- * iter->user_data Type NAMESPACE | COMMODITY | PRICE
- * iter->user_data2 A pointer to the namespace|commodity|item structure
- * iter->user_data3 The index of the item within its parent list
+ * In this model, valid paths take the form "X", "X:Y", or "X:Y:Z", where:
+ * X is an index into the namespaces list held by the commodity db
+ * Y is an index into the commodity list for the namespace
+ * Z is an index into the price list for the commodity
+ *
+ * Iterators are populated with the following private data:
+ * iter->user_data Type NAMESPACE | COMMODITY | PRICE
+ * iter->user_data2 A pointer to the namespace|commodity|item structure
+ * iter->user_data3 The index of the item within its parent list
*/
#include "config.h"
@@ -59,8 +65,18 @@
* the null string to be printed by the view. The removal kicks in
* immediately after the redraw and causes the blank line to be
* removed.
+ *
+ * Charles Day: I found that by the time the main loop is reached and
+ * the idle timer goes off, many qof events may have been generated and
+ * handled. In particular, a commodity could be removed, edited, and
+ * re-added by the security editor and all those events would happen
+ * before the timer goes off. This caused a problem where a re-added
+ * commodity would get whacked when the timer went off. I found that
+ * adding a check for pending removals at the beginning of the event
+ * handler fixes that problem and also resolves the race condition.
+ *
*/
-#undef RACE_CONDITION_SOLVED
+#define RACE_CONDITION_SOLVED
/** Static Globals *******************************************************/
static QofLogModule log_module = GNC_MOD_GUI;
@@ -138,7 +154,7 @@
0,
(GInstanceInitFunc) gnc_tree_model_price_init
};
-
+
static const GInterfaceInfo tree_model_info = {
(GInterfaceInitFunc) gnc_tree_model_price_tree_model_init,
NULL,
@@ -148,7 +164,7 @@
gnc_tree_model_price_type = g_type_register_static (GNC_TYPE_TREE_MODEL,
GNC_TREE_MODEL_PRICE_NAME,
&our_info, 0);
-
+
g_type_add_interface_static (gnc_tree_model_price_type,
GTK_TYPE_TREE_MODEL,
&tree_model_info);
@@ -330,7 +346,7 @@
g_return_val_if_fail (iter != NULL, NULL);
g_return_val_if_fail (iter->user_data != NULL, NULL);
g_return_val_if_fail (iter->stamp == model->stamp, NULL);
-
+
if (iter->user_data != ITER_IS_PRICE)
return NULL;
return (GNCPrice *)iter->user_data2;
@@ -340,10 +356,10 @@
/* Gnc Tree Model Debugging Utility Function */
/************************************************************/
-#define debug_path(fn, path) { \
- gchar *path_string = gtk_tree_path_to_string(path); \
- fn("tree path %s", path_string); \
- g_free(path_string); \
+#define debug_path(fn, path) { \
+ gchar *path_string = gtk_tree_path_to_string(path); \
+ fn("tree path %s", path_string? path_string : "(NULL)"); \
+ g_free(path_string); \
}
#define ITER_STRING_LEN 256
@@ -373,16 +389,16 @@
switch (GPOINTER_TO_INT(iter->user_data)) {
case GPOINTER_TO_INT(ITER_IS_NAMESPACE):
namespace = (gnc_commodity_namespace *) iter->user_data2;
- snprintf(string, ITER_STRING_LEN,
+ snprintf(string, ITER_STRING_LEN,
"[stamp:%x data:%d (NAMESPACE), %p (%s), %d]",
iter->stamp, GPOINTER_TO_INT(iter->user_data),
iter->user_data2, gnc_commodity_namespace_get_name (namespace),
GPOINTER_TO_INT(iter->user_data3));
break;
-
+
case GPOINTER_TO_INT(ITER_IS_COMMODITY):
commodity = (gnc_commodity *) iter->user_data2;
- snprintf(string, ITER_STRING_LEN,
+ snprintf(string, ITER_STRING_LEN,
"[stamp:%x data:%d (COMMODITY), %p (%s), %d]",
iter->stamp, GPOINTER_TO_INT(iter->user_data),
iter->user_data2, gnc_commodity_get_mnemonic (commodity),
@@ -392,7 +408,7 @@
case GPOINTER_TO_INT(ITER_IS_PRICE):
price= (GNCPrice *) iter->user_data2;
commodity = gnc_price_get_commodity(price);
- snprintf(string, ITER_STRING_LEN,
+ snprintf(string, ITER_STRING_LEN,
"[stamp:%x data:%d (PRICE), %p (%s:%s), %d]",
iter->stamp, GPOINTER_TO_INT(iter->user_data),
iter->user_data2, gnc_commodity_get_mnemonic (commodity),
@@ -401,7 +417,7 @@
break;
default:
- snprintf(string, ITER_STRING_LEN,
+ snprintf(string, ITER_STRING_LEN,
"[stamp:%x data:%d (UNKNOWN), %p, %d]",
iter->stamp,
GPOINTER_TO_INT(iter->user_data),
@@ -485,18 +501,12 @@
guint i, depth;
g_return_val_if_fail (GNC_IS_TREE_MODEL_PRICE (tree_model), FALSE);
-
+
depth = gtk_tree_path_get_depth (path);
- ENTER("model %p, iter, %p, path %p (depth %d)", tree_model, iter, path, depth);
+ ENTER("model %p, iter %p, path %p (depth %d)", tree_model, iter, path, depth);
debug_path(DEBUG, path);
- model = GNC_TREE_MODEL_PRICE (tree_model);
- priv = GNC_TREE_MODEL_PRICE_GET_PRIVATE(model);
- if (priv->price_db == NULL) {
- LEAVE("no price db");
- return FALSE;
- }
-
+ /* Check the path depth. */
if (depth == 0) {
LEAVE("depth too small");
return FALSE;
@@ -506,13 +516,26 @@
return FALSE;
}
+ /* Make sure the model has a price db. */
+ model = GNC_TREE_MODEL_PRICE (tree_model);
+ priv = GNC_TREE_MODEL_PRICE_GET_PRIVATE(model);
+ if (priv->price_db == NULL) {
+ LEAVE("no price db");
+ return FALSE;
+ }
+
+ /* Verify the first part of the path: the namespace. */
ct = qof_book_get_data (priv->book, GNC_COMMODITY_TABLE);
ns_list = gnc_commodity_table_get_namespaces_list(ct);
i = gtk_tree_path_get_indices (path)[0];
- g_return_val_if_fail (i >= 0 && i < g_list_length (ns_list), FALSE);
namespace = g_list_nth_data (ns_list, i);
+ if (!namespace) {
+ LEAVE("invalid path at namespace");
+ return FALSE;
+ }
if (depth == 1) {
+ /* Return an iterator for the namespace. */
iter->stamp = model->stamp;
iter->user_data = ITER_IS_NAMESPACE;
iter->user_data2 = namespace;
@@ -521,12 +544,17 @@
return TRUE;
}
+ /* Verify the second part of the path: the commodity. */
cm_list = gnc_commodity_namespace_get_commodity_list(namespace);
i = gtk_tree_path_get_indices (path)[1];
- g_return_val_if_fail (i >= 0 && i < g_list_length (cm_list), FALSE);
commodity = g_list_nth_data (cm_list, i);
+ if (!commodity) {
+ LEAVE("invalid path at commodity");
+ return FALSE;
+ }
if (depth == 2) {
+ /* Return an iterator for the commodity. */
iter->stamp = model->stamp;
iter->user_data = ITER_IS_COMMODITY;
iter->user_data2 = commodity;
@@ -535,17 +563,22 @@
return TRUE;
}
+ /* Verify the third part of the path: the price. */
price_list = gnc_pricedb_get_prices(priv->price_db, commodity, NULL);
i = gtk_tree_path_get_indices (path)[2];
+ price = g_list_nth_data (price_list, i);
+ gnc_price_list_destroy(price_list);
/* There's a race condition here that I can't resolve.
* Comment this check out for now, and we'll handle the
* resulting problem elsewhere. */
#ifdef RACE_CONDITION_SOLVED
- g_return_val_if_fail (i >= 0 && i < g_list_length (price_list), FALSE);
+ if (!price) {
+ LEAVE("invalid path at price");
+ return FALSE;
+ }
#endif
- price = g_list_nth_data (price_list, i);
- gnc_price_list_destroy(price_list);
+ /* Return an iterator for the price. */
iter->stamp = model->stamp;
iter->user_data = ITER_IS_PRICE;
iter->user_data2 = price;
@@ -571,26 +604,29 @@
g_return_val_if_fail (iter != NULL, NULL);
g_return_val_if_fail (iter->user_data != NULL, NULL);
g_return_val_if_fail (iter->stamp == model->stamp, NULL);
-
+
+ /* Make sure this model has a price db. */
priv = GNC_TREE_MODEL_PRICE_GET_PRIVATE(model);
if (priv->price_db == NULL) {
LEAVE("no price db");
return FALSE;
}
- /* Work through the namespace part */
if (iter->user_data == ITER_IS_NAMESPACE) {
+ /* Create a path to the namespace. This is just the index into
+ * the namespace list, which we already stored in user_data3. */
path = gtk_tree_path_new ();
gtk_tree_path_append_index (path, GPOINTER_TO_INT(iter->user_data3));
debug_path(LEAVE, path);
return path;
}
+ /* Get the namespaces list. */
ct = qof_book_get_data (priv->book, GNC_COMMODITY_TABLE);
ns_list = gnc_commodity_table_get_namespaces_list(ct);
- /* Work through the commodity part. */
if (iter->user_data == ITER_IS_COMMODITY) {
+ /* Create a path to the commodity. */
commodity = (gnc_commodity*)iter->user_data2;
namespace = gnc_commodity_get_namespace_ds(commodity);
path = gtk_tree_path_new ();
@@ -600,6 +636,7 @@
return path;
}
+ /* Create a path to the price. */
commodity = gnc_price_get_commodity((GNCPrice*)iter->user_data2);
namespace = gnc_commodity_get_namespace_ds(commodity);
cm_list = gnc_commodity_namespace_get_commodity_list(namespace);
@@ -854,7 +891,7 @@
iter->user_data2 = g_list_nth_data(list, 0);
iter->user_data3 = GINT_TO_POINTER(0);
gnc_price_list_destroy(list);
- LEAVE("cm iter %p (%s)", iter, iter_to_string(model, iter));
+ LEAVE("price iter %p (%s)", iter, iter_to_string(model, iter));
return TRUE;
}
@@ -941,7 +978,7 @@
list = gnc_pricedb_get_prices(priv->price_db, commodity, NULL);
n = g_list_length(list);
gnc_price_list_destroy(list);
- LEAVE("cm list length %d", n);
+ LEAVE("price list length %d", n);
return n;
}
@@ -961,13 +998,13 @@
gnc_commodity_namespace *namespace;
gnc_commodity *commodity;
GList *list;
-
+
g_return_val_if_fail (GNC_IS_TREE_MODEL_PRICE (tree_model), FALSE);
g_return_val_if_fail (iter != NULL, FALSE);
model = GNC_TREE_MODEL_PRICE (tree_model);
- ENTER("model %p, iter %p, parent %p (%s)",
- tree_model, iter, parent, iter_to_string(model, parent));
+ ENTER("model %p, iter %p, parent %p (%s), n %d",
+ tree_model, iter, parent, iter_to_string(model, parent), n);
priv = GNC_TREE_MODEL_PRICE_GET_PRIVATE(model);
if (parent == NULL) {
@@ -995,15 +1032,15 @@
}
if (parent->user_data == ITER_IS_COMMODITY) {
- commodity = (gnc_commodity *)iter->user_data2;
+ commodity = (gnc_commodity *)parent->user_data2;
list = gnc_pricedb_get_prices(priv->price_db, commodity, NULL);
iter->stamp = model->stamp;
- iter->user_data = ITER_IS_COMMODITY;
+ iter->user_data = ITER_IS_PRICE;
iter->user_data2 = g_list_nth_data(list, n);
iter->user_data3 = GINT_TO_POINTER(n);
gnc_price_list_destroy(list);
- LEAVE("cm iter %p (%s)", iter, iter_to_string(model, iter));
+ LEAVE("price iter %p (%s)", iter, iter_to_string(model, iter));
return iter->user_data2 != NULL;
}
@@ -1023,7 +1060,7 @@
gnc_commodity * commodity;
gnc_commodity_namespace *namespace;
GList *list;
-
+
g_return_val_if_fail (GNC_IS_TREE_MODEL_PRICE (tree_model), FALSE);
g_return_val_if_fail (iter != NULL, FALSE);
g_return_val_if_fail (child != NULL, FALSE);
@@ -1059,7 +1096,7 @@
iter->user_data = ITER_IS_COMMODITY;
iter->user_data2 = commodity;
iter->user_data3 = GINT_TO_POINTER(g_list_index(list, commodity));
- LEAVE("ns iter %p (%s)", iter, iter_to_string(model, iter));
+ LEAVE("cm iter %p (%s)", iter, iter_to_string(model, iter));
return TRUE;
}
@@ -1081,7 +1118,7 @@
gnc_commodity *commodity;
GList *list;
gint n;
-
+
ENTER("model %p, price %p, iter %p", model, price, iter);
g_return_val_if_fail (GNC_IS_TREE_MODEL_PRICE (model), FALSE);
g_return_val_if_fail ((price != NULL), FALSE);
@@ -1161,7 +1198,7 @@
gnc_commodity_namespace *namespace;
GList *list;
gint n;
-
+
ENTER("model %p, commodity %p, iter %p", model, commodity, iter);
g_return_val_if_fail (GNC_IS_TREE_MODEL_PRICE (model), FALSE);
g_return_val_if_fail ((commodity != NULL), FALSE);
@@ -1239,7 +1276,7 @@
gnc_commodity_table *ct;
GList *list;
gint n;
-
+
ENTER("model %p, namespace %p, iter %p", model, namespace, iter);
g_return_val_if_fail (GNC_IS_TREE_MODEL_PRICE (model), FALSE);
g_return_val_if_fail ((namespace != NULL), FALSE);
@@ -1306,73 +1343,138 @@
static GSList *pending_removals = NULL;
-/** This function performs common updating to the model after an
- * price has been added or removed. The parent entry needs to be
- * tapped on the shoulder so that it can correctly update the
- * disclosure triangle (first added child/last removed child) or
- * possibly rebuild its child list of that level of accounts is
- * visible.
+/** This function updates the model when a row is being added.
+ * The immediate parent needs to be tapped on the shoulder so that it
+ * can correctly update the disclosure triangle (if this is its first
+ * child.)
*
* @internal
*
- * @param model The price tree model containing the price
- * that has been added or deleted.
+ * @param model The price tree model.
*
- * @param path The path to the newly added item, or the just removed
- * item.
+ * @param iter The iterator of the new row.
*/
static void
-gnc_tree_model_price_path_added (GncTreeModelPrice *model,
- GtkTreeIter *iter)
+gnc_tree_model_price_row_add (GncTreeModelPrice *model,
+ GtkTreeIter *iter)
{
GtkTreePath *path;
GtkTreeModel *tree_model;
GtkTreeIter tmp_iter;
+ gint i;
ENTER("model %p, iter (%p)%s", model, iter, iter_to_string(model, iter));
+
+ /* We're adding a row, so the lists on which this model is based have
+ * changed. Since existing iterators (except the one just passed in)
+ * are all based on old indexes into those lists, we need to invalidate
+ * them, which we can do by changing the model's stamp. */
+ do {
+ model->stamp++;
+ } while (model->stamp == 0);
+ iter->stamp = model->stamp;
+
+ /* Tag the new row as inserted. */
tree_model = GTK_TREE_MODEL(model);
path = gnc_tree_model_price_get_path (tree_model, iter);
-
- /* Tag the new item as inserted. */
gtk_tree_model_row_inserted (tree_model, path, iter);
- /* */
- gtk_tree_path_up(path);
- while (gtk_tree_path_get_depth(path) != 0) {
- if (gtk_tree_model_get_iter(tree_model, &tmp_iter, path)) {
- gtk_tree_model_row_changed(tree_model, path, &tmp_iter);
- if (gtk_tree_model_iter_n_children(tree_model, &tmp_iter) == 1) {
- gtk_tree_model_row_has_child_toggled(tree_model, path, &tmp_iter);
- }
- }
- gtk_tree_path_up(path);
+ /* Inform all ancestors. */
+ /*
+ * Charles Day: I don't think calls to gtk_tree_model_row_changed() should
+ * be necessary. It is just a workaround for bug #540201.
+ */
+ if (gtk_tree_path_up(path) &&
+ gtk_tree_path_get_depth(path) > 0 &&
+ gtk_tree_model_get_iter(tree_model, &tmp_iter, path)) {
+ /* Signal the change to the parent. */
+ gtk_tree_model_row_changed(tree_model, path, &tmp_iter);
+
+ /* Is this the parent's first child? */
+ if (gtk_tree_model_iter_n_children(tree_model, &tmp_iter) == 1)
+ gtk_tree_model_row_has_child_toggled(tree_model, path, &tmp_iter);
+
+ /* Signal any other ancestors. */
+ while (gtk_tree_path_up(path) &&
+ gtk_tree_path_get_depth(path) > 0 &&
+ gtk_tree_model_get_iter(tree_model, &tmp_iter, path)) {
+ gtk_tree_model_row_changed(tree_model, path, &tmp_iter);
+ }
}
gtk_tree_path_free(path);
- do {
- model->stamp++;
- } while (model->stamp == 0);
+ /* If the new row already has children, signal that so the expander
+ * can be shown. This can happen, for example, if a namespace or
+ * commodity is changed in another place (like the Security Editor)
+ * and gets removed and then re-added to the commodity db. */
+ if (gnc_tree_model_price_iter_has_child(tree_model, iter))
+ {
+ path = gnc_tree_model_price_get_path(tree_model, iter);
+ gtk_tree_model_row_has_child_toggled(tree_model, path, iter);
+ gtk_tree_path_free(path);
+ }
+
LEAVE(" ");
}
+/** This function updates the model when a row is being deleted.
+ * The immediate parent needs to be tapped on the shoulder so that it
+ * can correctly update the disclosure triangle (if this was its last
+ * child.)
+ *
+ * @internal
+ *
+ * @param model The price tree model that is losing a row.
+ *
+ * @param path The path of the row being removed.
+ */
static void
-gnc_tree_model_price_path_deleted (GncTreeModelPrice *model,
- GtkTreePath *path)
+gnc_tree_model_price_row_delete (GncTreeModelPrice *model,
+ GtkTreePath *path)
{
+ GtkTreeModel *tree_model;
GtkTreeIter iter;
+ g_return_if_fail(GNC_IS_TREE_MODEL_PRICE(model));
+ g_return_if_fail(path);
+
debug_path(ENTER, path);
- while (gtk_tree_path_up(path) && (gtk_tree_path_get_depth(path) > 0)) {
- debug_path(DEBUG, path);
- if (gtk_tree_model_get_iter (GTK_TREE_MODEL(model), &iter, path)) {
+
+ tree_model = GTK_TREE_MODEL(model);
+
+ /* We're removing a row, so the lists on which this model is based have
+ * changed. Since existing iterators are all based on old indexes into
+ * those lists, we need to invalidate them, which we can do by changing
+ * the model's stamp. */
+ do {
+ model->stamp++;
+ } while (model->stamp == 0);
+
+ /* Signal that the path has been deleted. */
+ gtk_tree_model_row_deleted(tree_model, path);
+
+ /* Issue any appropriate signals to ancestors. */
+ if (gtk_tree_path_up(path) &&
+ gtk_tree_path_get_depth(path) > 0 &&
+ gtk_tree_model_get_iter(tree_model, &iter, path)) {
+ DEBUG("iter %s", iter_to_string(model, &iter));
+
+ /* Signal the change to the parent. */
+ gtk_tree_model_row_changed(tree_model, path, &iter);
+
+ /* Was this the parent's only child? */
+ if (!gtk_tree_model_iter_has_child(tree_model, &iter))
+ gtk_tree_model_row_has_child_toggled(tree_model, path, &iter);
+
+ /* Signal any other ancestors. */
+ while (gtk_tree_path_up(path) &&
+ gtk_tree_path_get_depth(path) > 0 &&
+ gtk_tree_model_get_iter(tree_model, &iter, path)) {
DEBUG("iter %s", iter_to_string(model, &iter));
- gtk_tree_model_row_changed (GTK_TREE_MODEL(model), path, &iter);
+ gtk_tree_model_row_changed(tree_model, path, &iter);
}
}
- do {
- model->stamp++;
- } while (model->stamp == 0);
LEAVE(" ");
}
@@ -1382,8 +1484,8 @@
* each time an item is removed from the model. This function will
* be called as an idle function some time after the user requests
* the deletion. (Most likely when the event handler for the "OK"
- * button click returns. This function will send the "row_deleted"
- * signal to any/all parent models for each entry deleted.
+ * button click returns.) This function will send the "row_deleted"
+ * signal to the model for each entry deleted.
*
* @internal
*
@@ -1396,49 +1498,60 @@
static gboolean
gnc_tree_model_price_do_deletions (gpointer unused)
{
- GSList *iter, *next = NULL;
- remove_data *data;
+ ENTER(" ");
- for (iter = pending_removals; iter != NULL; iter = next) {
- next = g_slist_next(iter);
- data = iter->data;
- pending_removals = g_slist_delete_link (pending_removals, iter);
+ /* Go through the list of paths needing removal. */
+ while (pending_removals)
+ {
+ remove_data *data = pending_removals->data;
+ pending_removals = g_slist_delete_link(pending_removals, pending_removals);
- gtk_tree_model_row_deleted (GTK_TREE_MODEL(data->model), data->path);
- gnc_tree_model_price_path_deleted (data->model, data->path);
- gtk_tree_path_free(data->path);
- g_free(data);
+ if (data)
+ {
+ debug_path(DEBUG, data->path);
+
+ /* Remove the path. */
+ gnc_tree_model_price_row_delete(data->model, data->path);
+
+ gtk_tree_path_free(data->path);
+ g_free(data);
+ }
}
- /* Remove me */
+ LEAVE(" ");
+ /* Don't call me again. */
return FALSE;
}
-/** This function is the handler for all event messages from the
- * engine. Its purpose is to update the price tree model any
- * time a price or namespace is added to the engine or deleted
- * from the engine. This change to the model is then propagated to
- * any/all overlying filters and views. This function listens to the
- * ADD, REMOVE, and DESTROY events.
+/** This function is the handler for all event messages from the engine.
+ * Its purpose is to update the tree model any time a price, commodity, or
+ * namespace is added to the engine, modified, or deleted from the engine.
+ * This change to the model is then propagated to any/all overlying filters
+ * and views. This function listens to the ADD, REMOVE, MODIFY, and DESTROY
+ * events.
*
* @internal
*
- * @warning There is a "Catch 22" situation here.
- * gtk_tree_model_row_deleted() can't be called until after the item
- * has been deleted from the real model (which is the engine's
- * price table for us), but once the price has been deleted
+ * @warning There is a "Catch 22" situation here. The REMOVE event
+ * indicates that a namespace, commodity, or price is *about* to be
+ * removed. But we can't actually delete the row by calling
+ * gtk_tree_model_row_deleted() until after the item has *actually*
+ * been removed from the real model (which is the engine's commodity
+ * and price db's for us). But once the item has been deleted
* from the engine we have no way to determine the path to pass to
- * row_deleted(). This is a PITA, but the only other choice is to
- * have this model mirror the engine's price table instead of
+ * row_deleted(). So we will save the path now, then call a function
+ * to delete the row when we receive the next event or we're idle
+ * (whichever comes first.) This is a PITA, but the only other choice
+ * is to have this model mirror the engine's price table instead of
* referencing it directly.
*
* @param entity The affected item.
*
* @param event type The type of the event. This function only cares
- * about items of type ADD, REMOVE, and DESTROY.
+ * about items of type ADD, REMOVE, MODIFY, and DESTROY.
*
- * @param user_data A pointer to the account tree model.
+ * @param user_data A pointer to the tree model.
*
* @param event_data A pointer to additional data about this event.
*/
@@ -1458,6 +1571,10 @@
entity, event_type, user_data, event_data);
model = (GncTreeModelPrice *)user_data;
+ /* Do deletions if any are pending. */
+ if (pending_removals)
+ gnc_tree_model_price_do_deletions(NULL);
+
/* hard failures */
g_return_if_fail(GNC_IS_TREE_MODEL_PRICE(model));
@@ -1503,7 +1620,7 @@
case QOF_EVENT_ADD:
/* Tell the filters/views where the new account was added. */
DEBUG("add %s", name);
- gnc_tree_model_price_path_added (model, &iter);
+ gnc_tree_model_price_row_add (model, &iter);
break;
case QOF_EVENT_REMOVE:
@@ -1521,6 +1638,7 @@
pending_removals = g_slist_append (pending_removals, data);
g_idle_add_full(G_PRIORITY_HIGH_IDLE,
gnc_tree_model_price_do_deletions, NULL, NULL);
+
LEAVE(" ");
return;
More information about the gnucash-changes
mailing list