From 08deb7154199c31b68e0ba8cf449f33a74f0ba21 Mon Sep 17 00:00:00 2001 From: Charles Day Date: Fri, 16 May 2008 22:14:25 +0000 Subject: [PATCH] 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 git-svn-id: svn+ssh://svn.gnucash.org/repo/gnucash/trunk@17157 57a11ea4-9604-0410-9ed3-97b8803252fd --- .../qif-import/dialog-account-picker.c | 134 ++++++++++++------ .../qif-import/dialog-account-picker.h | 2 +- .../qif-import/druid-qif-import.c | 11 +- 3 files changed, 98 insertions(+), 49 deletions(-) diff --git a/src/import-export/qif-import/dialog-account-picker.c b/src/import-export/qif-import/dialog-account-picker.c index ba7a23f009..b5f9f607b7 100644 --- a/src/import-export/qif-import/dialog-account-picker.c +++ b/src/import-export/qif-import/dialog-account-picker.c @@ -50,12 +50,23 @@ struct _accountpickerdialog { 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 @@ acct_tree_add_accts(SCM accts, /* 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 @@ acct_tree_add_accts(SCM accts, -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 @@ acct_tree_add_accts(SCM accts, 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 @@ acct_tree_add_accts(SCM accts, } } + +/**************************************************************** + * 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 @@ build_acct_tree(QIFAccountPickerDialog * picker, QIFImportWindow * import) } } + +/**************************************************************** + * 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 @@ gnc_ui_qif_account_picker_map_cb(GtkWidget * w, gpointer user_data) * 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,12 +323,6 @@ qif_account_picker_dialog(QIFImportWindow * qif_wind, SCM map_entry) 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; @@ -335,7 +382,10 @@ qif_account_picker_dialog(QIFImportWindow * qif_wind, SCM map_entry) 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; } diff --git a/src/import-export/qif-import/dialog-account-picker.h b/src/import-export/qif-import/dialog-account-picker.h index 978bbe102e..b79bbf3835 100644 --- a/src/import-export/qif-import/dialog-account-picker.h +++ b/src/import-export/qif-import/dialog-account-picker.h @@ -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; diff --git a/src/import-export/qif-import/druid-qif-import.c b/src/import-export/qif-import/druid-qif-import.c index 61b4209bf1..fa1916576c 100644 --- a/src/import-export/qif-import/druid-qif-import.c +++ b/src/import-export/qif-import/druid-qif-import.c @@ -1224,8 +1224,7 @@ select_line(QIFImportWindow *wind, GtkTreeSelection *selection, 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 @@ gnc_ui_qif_import_comm_next_cb(GnomeDruidPage * page, 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 @@ gnc_ui_qif_import_druid_make(void) 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; }