AUDIT: r17157 - gnucash/trunk/src/import-export/qif-import - Bug #514210: This fixes a QIF import bug introduced in r17156 which prevented users from creating new accounts while mapping accounts. It also fixes several GUI annoyances:

Charles Day cedayiv at cvs.gnucash.org
Fri May 16 18:14:25 EDT 2008


Author: cedayiv
Date: 2008-05-16 18:14:25 -0400 (Fri, 16 May 2008)
New Revision: 17157
Trac: http://svn.gnucash.org/trac/changeset/17157

Modified:
   gnucash/trunk/src/import-export/qif-import/dialog-account-picker.c
   gnucash/trunk/src/import-export/qif-import/dialog-account-picker.h
   gnucash/trunk/src/import-export/qif-import/druid-qif-import.c
Log:
Bug #514210: This fixes a QIF import bug introduced in r17156 which prevented users from creating new accounts while mapping accounts. It also fixes several GUI annoyances:
 1. The tree now automatically expands to show the currently selected account.
 2. The new account dialog's OK button is now activated by the Enter key.
 3. Focus returns to the account tree when the new account dialog is closed.
 4. Creation of empty account names is prevented.

Finally, a memory leak has been fixed and many new comments have been added.
BP


Modified: gnucash/trunk/src/import-export/qif-import/dialog-account-picker.c
===================================================================
--- gnucash/trunk/src/import-export/qif-import/dialog-account-picker.c	2008-05-14 02:49:29 UTC (rev 17156)
+++ gnucash/trunk/src/import-export/qif-import/dialog-account-picker.c	2008-05-16 22:14:25 UTC (rev 17157)
@@ -50,12 +50,23 @@
   gchar           * selected_name;
 };
 
+
+/****************************************************************
+ * acct_tree_add_accts
+ *
+ * Given a Scheme list of accounts, this function populates a
+ * GtkTreeStore from them. If the search_name and reference
+ * parameters are provided, and an account is found whose full
+ * name matches search_name, then a GtkTreeRowReference* will be
+ * returned in the reference parameter.
+ ****************************************************************/
+
 static void
 acct_tree_add_accts(SCM accts,
                     GtkTreeStore *store,
                     GtkTreeIter *parent,
                     const char *base_name,
-                    const char *selected_name,
+                    const char *search_name,
                     GtkTreeRowReference **reference)
 {
   GtkTreeIter  iter;
@@ -88,8 +99,8 @@
 
     /* compute full name */
     if (base_name && *base_name) {
-      acctname =  g_strjoin(gnc_get_account_separator_string(),
-                            base_name, compname, (char *)NULL);
+      acctname = g_strjoin(gnc_get_account_separator_string(),
+                           base_name, compname, (char *)NULL);
     }
     else {
       acctname = g_strdup(compname);
@@ -105,7 +116,7 @@
                        -1);
 
     if (reference && !*reference &&
-        selected_name && (g_utf8_collate(selected_name, acctname) == 0)) {
+        search_name && (g_utf8_collate(search_name, acctname) == 0)) {
       GtkTreePath *path = gtk_tree_model_get_path(GTK_TREE_MODEL(store), &iter);
       *reference = gtk_tree_row_reference_new(GTK_TREE_MODEL(store), path);
       gtk_tree_path_free(path);
@@ -113,7 +124,7 @@
 
     if(!leafnode) {
       acct_tree_add_accts(SCM_CADDR(current), store, &iter, acctname,
-                          selected_name, reference);
+                          search_name, reference);
     }
 
     g_free(acctname);
@@ -122,27 +133,45 @@
   }
 }
 
+
+/****************************************************************
+ * build_acct_tree
+ *
+ * This function refreshes the contents of the account tree.
+ ****************************************************************/
+
 static void
 build_acct_tree(QIFAccountPickerDialog * picker, QIFImportWindow * import)
 {
   SCM  get_accts = scm_c_eval_string("qif-import:get-all-accts");
-  SCM  acct_tree = scm_call_1(get_accts,
-                              gnc_ui_qif_import_druid_get_mappings(import));
+  SCM  acct_tree;
   GtkTreeStore *store;
   GtkTreePath *path;
   GtkTreeSelection* selection;
   GtkTreeRowReference *reference = NULL;
+  gchar *name_to_select;
 
+  g_return_if_fail(picker && import);
+
+  /* Get an account tree with all existing and to-be-imported accounts. */
+  acct_tree = scm_call_1(get_accts,
+                         gnc_ui_qif_import_druid_get_mappings(import));
+
+  /* Rebuild the store.
+   * NOTE: It is necessary to save a copy of the name to select, because
+   *       when the store is cleared, everything becomes unselected. */
+  name_to_select = g_strdup(picker->selected_name);
   store = GTK_TREE_STORE(gtk_tree_view_get_model(picker->treeview));
   gtk_tree_store_clear(store);
+  acct_tree_add_accts(acct_tree, store, NULL, NULL, name_to_select, &reference);
+  g_free(name_to_select);
 
-  acct_tree_add_accts(acct_tree, store, NULL, NULL,
-                      picker->selected_name, &reference);
-
+  /* Select and display the indicated account (if it was found). */
   if (reference) {
     selection = gtk_tree_view_get_selection(picker->treeview);
     path = gtk_tree_row_reference_get_path(reference);
     if (path) {
+      gtk_tree_view_expand_to_path(picker->treeview, path);
       gtk_tree_selection_select_path(selection, path);
       gtk_tree_path_free(path);
     }
@@ -150,45 +179,61 @@
   }
 }
 
+
+/****************************************************************
+ * gnc_ui_qif_account_picker_new_cb
+ *
+ * This handler is invoked when the user wishes to create a new
+ * account.
+ ****************************************************************/
+
 static void
 gnc_ui_qif_account_picker_new_cb(GtkButton * w, gpointer user_data)
 {
   QIFAccountPickerDialog * wind = user_data;
   SCM name_setter = scm_c_eval_string("qif-map-entry:set-gnc-name!");
-  const char *name;
-  int  response;
-  char * fullname;
+  const gchar *name;
+  int response;
+  gchar *fullname;
   GtkWidget *dlg, *entry;
 
+  /* Create a dialog to get the new account name. */
   dlg = gtk_message_dialog_new(GTK_WINDOW(wind->dialog),
-                                GTK_DIALOG_DESTROY_WITH_PARENT,
-                                GTK_MESSAGE_QUESTION,
-                                GTK_BUTTONS_OK_CANCEL,
-                                "%s", _("Enter a name for the account"));
-
+                               GTK_DIALOG_DESTROY_WITH_PARENT,
+                               GTK_MESSAGE_QUESTION,
+                               GTK_BUTTONS_OK_CANCEL,
+                               "%s", _("Enter a name for the account"));
+  gtk_dialog_set_default_response(GTK_DIALOG(dlg), GTK_RESPONSE_OK);
   entry = gtk_entry_new();
+  gtk_entry_set_activates_default(GTK_ENTRY(entry), TRUE);
   gtk_entry_set_max_length(GTK_ENTRY(entry), 250);
   gtk_widget_show(entry);
   gtk_container_add(GTK_CONTAINER(GTK_DIALOG(dlg)->vbox), entry);
 
+  /* Run the dialog to get the new account name. */
   response = gtk_dialog_run(GTK_DIALOG(dlg));
-  if (response == GTK_RESPONSE_OK) {
-    name = gtk_entry_get_text(GTK_ENTRY(entry));
-    if(wind->selected_name && (strlen(wind->selected_name) > 0)) {
+  name = gtk_entry_get_text(GTK_ENTRY(entry));
+
+  /* Did the user enter a name and click OK? */
+  if (response == GTK_RESPONSE_OK && name && *name) {
+    /* If an account is selected, this will be a new subaccount. */
+    if(wind->selected_name && *(wind->selected_name))
+      /* We have the short name; determine the full name. */
       fullname = g_strjoin(gnc_get_account_separator_string(),
                            wind->selected_name, name, (char *)NULL);
-    }
-    else {
+    else
       fullname = g_strdup(name);
-    }
-    wind->selected_name = g_strdup(fullname);
+
+    /* Save the full name and update the map entry. */
+    g_free(wind->selected_name);
+    wind->selected_name = fullname;
     scm_call_2(name_setter, wind->map_entry, scm_makfrom0str(fullname));
-    g_free(fullname);
   }
   gtk_widget_destroy(dlg);
 
+  /* Refresh the tree display and give it the focus. */
   build_acct_tree(wind, wind->qif_wind);
-
+  gtk_widget_grab_focus(GTK_WIDGET(wind->treeview));
 }
 
 static void
@@ -241,24 +286,32 @@
  * qif_account_picker_dialog
  *
  * Select an account from the ones that the engine knows about,
- * plus those that will be created by the QIF import.  Returns
- * a new Scheme map entry, or SCM_BOOL_F on cancel. Modal.
+ * plus those that will be created by the QIF import.  If the
+ * user clicks OK, map_entry is changed and TRUE is returned.
+ * If the clicks Cancel instead, FALSE is returned. Modal.
  ****************************************************************/
 
-SCM
+gboolean
 qif_account_picker_dialog(QIFImportWindow * qif_wind, SCM map_entry)
 {
   QIFAccountPickerDialog * wind;
-  SCM clone_entry  = scm_c_eval_string("qif-map-entry:clone");
-  SCM init_pick    = scm_c_eval_string("qif-map-entry:gnc-name");
-  SCM new_entry    = scm_call_1(clone_entry, map_entry);
+  SCM gnc_name     = scm_c_eval_string("qif-map-entry:gnc-name");
+  SCM set_gnc_name = scm_c_eval_string("qif-map-entry:set-gnc-name!");
+  SCM orig_acct    = scm_call_1(gnc_name, map_entry);
   int response;
-  const gchar * scmname;
   GladeXML *xml;
   GtkWidget *button;
 
   wind = g_new0(QIFAccountPickerDialog, 1);
 
+  /* Save the map entry. */
+  wind->map_entry = map_entry;
+  scm_gc_protect_object(wind->map_entry);
+
+  /* Set the initial account to be selected. */
+  wind->selected_name = g_strdup(SCM_STRING_CHARS(orig_acct));
+
+
   xml = gnc_glade_xml_new("qif.glade", "QIF Import Account Picker");
 
   glade_xml_signal_connect_data(xml,
@@ -270,13 +323,7 @@
   wind->treeview   = GTK_TREE_VIEW(glade_xml_get_widget(xml, "account_tree"));
   wind->qif_wind   = qif_wind;
 
-  wind->map_entry  = new_entry;
 
-  scmname = SCM_STRING_CHARS(scm_call_1(init_pick, new_entry));
-  wind->selected_name = g_strdup(scmname);
-
-  scm_gc_protect_object(wind->map_entry);
-
   {
     GtkTreeStore *store;
     GtkCellRenderer *renderer;
@@ -335,7 +382,10 @@
   g_free(wind);
 
   if (response == GTK_RESPONSE_OK)
-    return new_entry;
+    return TRUE;
 
-  return SCM_BOOL_F;
+  /* Restore the original mapping. */
+  scm_call_2(set_gnc_name, map_entry, orig_acct);
+
+  return FALSE;
 }

Modified: gnucash/trunk/src/import-export/qif-import/dialog-account-picker.h
===================================================================
--- gnucash/trunk/src/import-export/qif-import/dialog-account-picker.h	2008-05-14 02:49:29 UTC (rev 17156)
+++ gnucash/trunk/src/import-export/qif-import/dialog-account-picker.h	2008-05-16 22:14:25 UTC (rev 17157)
@@ -28,7 +28,7 @@
 
 #include "druid-qif-import.h"
 
-SCM qif_account_picker_dialog(QIFImportWindow * wind, SCM initial_sel);
+gboolean qif_account_picker_dialog(QIFImportWindow * wind, SCM initial_sel);
 
 typedef struct _accountpickerdialog QIFAccountPickerDialog;
 

Modified: gnucash/trunk/src/import-export/qif-import/druid-qif-import.c
===================================================================
--- gnucash/trunk/src/import-export/qif-import/druid-qif-import.c	2008-05-14 02:49:29 UTC (rev 17156)
+++ gnucash/trunk/src/import-export/qif-import/druid-qif-import.c	2008-05-16 22:14:25 UTC (rev 17157)
@@ -1224,8 +1224,7 @@
   map_entry = scm_list_ref(display_info, scm_int2num(row));
 
   /* Call the account picker to update it. */
-  map_entry = qif_account_picker_dialog(wind, map_entry);
-  if (map_entry == SCM_BOOL_F)
+  if (!qif_account_picker_dialog(wind, map_entry))
     return;
   gnc_name = scm_call_1(get_gnc_name, map_entry);
 
@@ -1901,7 +1900,7 @@
       wind->new_namespaces = g_list_prepend(wind->new_namespaces, namespace);
     else
     {
-      g_warning("QIF import: Couldn't create namespace %s\n", namespace);
+      g_warning("QIF import: Couldn't create namespace %s", namespace);
       g_free(namespace);
     }
   }
@@ -2547,13 +2546,13 @@
   retval->show_doc_pages =
     gnc_gconf_get_bool(GCONF_SECTION, GCONF_NAME_SHOW_DOC, &err);
   if (err != NULL) {
-    g_warning("QIF import: gnc_gconf_get_bool error: %s\n", err->message);
+    g_warning("QIF import: gnc_gconf_get_bool error: %s", err->message);
     g_error_free(err);
 
     /* Show documentation pages by default. */
-    g_warning("QIF import: Couldn't get %s setting from gconf.\n",
+    g_warning("QIF import: Couldn't get %s setting from gconf.",
               GCONF_NAME_SHOW_DOC);
-    g_warning("QIF import: Documentation pages will be shown by default.\n");
+    g_warning("QIF import: Documentation pages will be shown by default.");
     retval->show_doc_pages = TRUE;
   }
 



More information about the gnucash-changes mailing list