Attachment reminder cleanup. Fixes 502312. Added some more fixes over Milan Crha's work.

svn path=/trunk/; revision=34700

2 files changed, 94 insertions, 77 deletions

diff --git a/plugins/attachment-reminder/ChangeLog b/plugins/attachment-reminder/ChangeLog
index f89ec0e4fe..014dba5554 100644
--- a/plugins/attachment-reminder/ChangeLog
+++ b/plugins/attachment-reminder/ChangeLog
@@ -1,3 +1,19 @@
+2007-12-15  Johnny Jacob  <jjohnny@novell.com>
+
+	** Cleanup of attachment reminder over Milan Crha's work.
+	
+	* attachment-reminder.c (check_for_attachment_clues): Unref gconf_client.
+	(commit_changes): reuse ui->gconf.
+	(e_plugin_lib_get_configure_widget): Remove unnecessary 
+	gconf_client_get_default ()
+
+2007-12-07  Milan Crha  <mcrha@redhat.com>
+
+	** Fix for bug #502312
+
+	* attachment-reminder.c: A little cleanup of configuration part of
+	the plugin to not use global variables and free memory properly.
+
 2007-12-05  Sankar P  <psankar@novell.com>
 
 	* attachment-reminder.c: (e_plugin_lib_get_configure_widget):
diff --git a/plugins/attachment-reminder/attachment-reminder.c b/plugins/attachment-reminder/attachment-reminder.c
index 06ea26e2a5..6bbf4d0e00 100644
--- a/plugins/attachment-reminder/attachment-reminder.c
+++ b/plugins/attachment-reminder/attachment-reminder.c
@@ -50,12 +50,14 @@
 #define GCONF_KEY_ATTACH_REMINDER_CLUES "/apps/evolution/mail/attachment_reminder_clues"
 
 typedef struct {
+	GladeXML *xml;
 	GConfClient *gconf;
 	GtkWidget   *treeview;
 	GtkWidget   *clue_add;
 	GtkWidget   *clue_edit;
 	GtkWidget   *clue_remove;
 	GtkWidget   *clue_container;
+	GtkListStore *store;
 } UIData;
 
 
@@ -79,10 +81,8 @@ static void commit_changes (UIData *ui);
 
 static void  cell_edited_callback (GtkCellRendererText *cell, gchar *path_string,
 				   gchar *new_text,UIData *ui);
-gboolean clue_foreach_check_isempty (GtkTreeModel *model, GtkTreePath
-					*path, GtkTreeIter *iter, gpointer data);
-
-static GtkListStore *store = NULL;
+static gboolean clue_foreach_check_isempty (GtkTreeModel *model, GtkTreePath
+					*path, GtkTreeIter *iter, UIData *ui);
 
 int
 e_plugin_lib_enable (EPluginLib *ep, int enable)
@@ -90,18 +90,17 @@ e_plugin_lib_enable (EPluginLib *ep, int enable)
 	return 0;
 }
 
-void org_gnome_evolution_attachment_reminder (EPlugin *ep, EMEventTargetComposer *t)
+void
+org_gnome_evolution_attachment_reminder (EPlugin *ep, EMEventTargetComposer *t)
 {
 	GConfClient *gconf;
 	char *rawstr = NULL, *filtered_str = NULL;
 
 	gconf = gconf_client_get_default ();
-	if (!gconf_client_get_bool (gconf, GCONF_KEY_ATTACHMENT_REMINDER, NULL))
-	{
+	if (!gconf_client_get_bool (gconf, GCONF_KEY_ATTACHMENT_REMINDER, NULL)){
 		g_object_unref (gconf);
 		return;
-	}
-	else
+	} else
 		g_object_unref (gconf);
 
 	rawstr = g_strdup (e_msg_composer_get_raw_message_text (t->composer));
@@ -116,17 +115,17 @@ void org_gnome_evolution_attachment_reminder (EPlugin *ep, EMEventTargetComposer
 			g_object_set_data ((GObject *) t->composer, "presend_check_status", GINT_TO_POINTER(1));
 
 	g_free (filtered_str);
-
-	return ;
 }
 
-static gboolean ask_for_missing_attachment (GtkWindow *window)
+static gboolean
+ask_for_missing_attachment (GtkWindow *window)
 {
 	return em_utils_prompt_user(window, GCONF_KEY_ATTACHMENT_REMINDER ,"org.gnome.evolution.plugins.attachment_reminder:attachment-reminder", NULL);
 }
 
 /* check for the clues */
-static gboolean check_for_attachment_clues (gchar *msg)
+static gboolean
+check_for_attachment_clues (gchar *msg)
 {
 	/* TODO : Add more strings. RegEx ??? */
 
@@ -140,6 +139,8 @@ static gboolean check_for_attachment_clues (gchar *msg)
 	/* Get the list from gconf */
 	clue_list = gconf_client_get_list ( gconf, GCONF_KEY_ATTACH_REMINDER_CLUES, GCONF_VALUE_STRING, NULL );
 
+	g_object_unref (gconf);
+
 	msg_length = strlen (msg);
 
 	for (list = clue_list;list && !ret_val;list=g_slist_next(list)) {
@@ -159,7 +160,8 @@ static gboolean check_for_attachment_clues (gchar *msg)
 }
 
 /* check for the any attachment */
-static gboolean check_for_attachment (EMsgComposer *composer)
+static gboolean
+check_for_attachment (EMsgComposer *composer)
 {
 	EAttachmentBar* bar = (EAttachmentBar*)e_msg_composer_get_attachment_bar (composer);
 
@@ -169,17 +171,16 @@ static gboolean check_for_attachment (EMsgComposer *composer)
 	return FALSE;
 }
 
-static gchar* strip_text_msg (gchar *msg)
+static gchar*
+strip_text_msg (gchar *msg)
 {
 	gchar **lines = g_strsplit ( msg, "\n", -1);
 	gchar *stripped_msg = g_strdup (" ");
 
 	guint i=0;
 
-	while (lines [i])
-	{
-		if (lines [i] != NULL && !g_str_has_prefix (g_strstrip(lines[i]), ">"))
-		{
+	while (lines [i]){
+		if (lines [i] != NULL && !g_str_has_prefix (g_strstrip(lines[i]), ">")){
 			gchar *temp = stripped_msg;
 			stripped_msg = g_strconcat (" ", stripped_msg, lines[i], NULL);
 
@@ -200,7 +201,6 @@ commit_changes (UIData *ui)
 	GSList *clue_list = NULL;
 	GtkTreeIter iter;
 	gboolean valid;
-	GConfClient *client;
 
 	model = gtk_tree_view_get_model (GTK_TREE_VIEW (ui->treeview));
 	valid = gtk_tree_model_get_iter_first (model, &iter);
@@ -215,8 +215,7 @@ commit_changes (UIData *ui)
 		valid = gtk_tree_model_iter_next (model, &iter);
 	}
 
-	client = gconf_client_get_default ();
-	gconf_client_set_list (client, GCONF_KEY_ATTACH_REMINDER_CLUES, GCONF_VALUE_STRING, clue_list, NULL);
+	gconf_client_set_list (ui->gconf, GCONF_KEY_ATTACH_REMINDER_CLUES, GCONF_VALUE_STRING, clue_list, NULL);
 
 	g_slist_foreach (clue_list, (GFunc) g_free, NULL);
 	g_slist_free (clue_list);
@@ -226,55 +225,58 @@ static void
 clue_check_isempty (GtkTreeModel *model, GtkTreePath *path, GtkTreeIter *iter, UIData *ui)
 {
 	GtkTreeSelection *selection;
-	char *keyword;
+	char *keyword = NULL;
 	gboolean valid;
 
 	selection = gtk_tree_view_get_selection (GTK_TREE_VIEW (ui->treeview));
 	/* move to the previous node */
-	valid = gtk_tree_path_prev(path);
+	valid = gtk_tree_path_prev (path);
 
-	gtk_tree_model_get(model, iter, CLUE_KEYWORD_COLUMN, &keyword, -1);
-	if ((keyword) && !(g_utf8_strlen(g_strstrip(keyword), -1) > 0))
-		gtk_list_store_remove(store, iter);
+	gtk_tree_model_get (model, iter, CLUE_KEYWORD_COLUMN, &keyword, -1);
+	if ((keyword) && !(g_utf8_strlen (g_strstrip (keyword), -1) > 0))
+		gtk_list_store_remove (ui->store, iter);
 
 	/* Check if we have a valid row to select. If not, then select
 	 * the previous row */
-	if (gtk_list_store_iter_is_valid(GTK_LIST_STORE(model), iter)) {
+	if (gtk_list_store_iter_is_valid (GTK_LIST_STORE (model), iter)) {
 		gtk_tree_selection_select_iter (selection, iter);
 	} else {
 		if (path && valid) {
-			gtk_tree_model_get_iter(model, iter, path);
+			gtk_tree_model_get_iter (model, iter, path);
 			gtk_tree_selection_select_iter (selection, iter);
 		}
 	}
 
-	gtk_widget_grab_focus(ui->treeview);
+	gtk_widget_grab_focus (ui->treeview);
+	g_free (keyword);
 }
 
-gboolean
-clue_foreach_check_isempty (GtkTreeModel *model, GtkTreePath *path, GtkTreeIter *iter, gpointer data)
+static gboolean
+clue_foreach_check_isempty (GtkTreeModel *model, GtkTreePath *path, GtkTreeIter *iter, UIData *ui)
 {
 	gboolean valid;
-	char *keyword;
 
-	valid = gtk_tree_model_get_iter_first(model, iter);
-	while (valid && gtk_list_store_iter_is_valid(store, iter)) {
-		gtk_tree_model_get(model, iter, CLUE_KEYWORD_COLUMN, &keyword, -1);
+	valid = gtk_tree_model_get_iter_first (model, iter);
+	while (valid && gtk_list_store_iter_is_valid (ui->store, iter)) {
+		char *keyword = NULL;
+		gtk_tree_model_get (model, iter, CLUE_KEYWORD_COLUMN, &keyword, -1);
 		/* Check if the keyword is not empty and then emit the row-changed
 		signal (if we delete the row, then the iter gets corrupted) */
-		if ((keyword) && !(g_utf8_strlen(g_strstrip(keyword), -1) > 0))
-			gtk_tree_model_row_changed(model, path, iter);
+		if ((keyword) && !(g_utf8_strlen (g_strstrip (keyword), -1) > 0))
+			gtk_tree_model_row_changed (model, path, iter);
+
+		g_free (keyword);
 		valid = gtk_tree_model_iter_next (model, iter);
 	}
 
-	g_free(keyword);
 	return FALSE;
 }
 
-static void  cell_edited_callback (GtkCellRendererText *cell,
-                                  gchar               *path_string,
-                                  gchar               *new_text,
-                                  UIData             *ui)
+static void
+cell_edited_callback (GtkCellRendererText *cell,
+		      gchar               *path_string,
+		      gchar               *new_text,
+		      UIData             *ui)
 {
 	GtkTreeModel *model;
 	GtkTreeIter iter;
@@ -299,7 +301,7 @@ clue_add_clicked (GtkButton *button, UIData *ui)
 	GtkTreePath *path;
 
 	model = gtk_tree_view_get_model (GTK_TREE_VIEW (ui->treeview));
-	gtk_tree_model_foreach(model, clue_foreach_check_isempty, NULL);
+	gtk_tree_model_foreach (model, (GtkTreeModelForeachFunc) clue_foreach_check_isempty, ui);
 
 	/* Disconnect from signal so that we can create an empty row */
 	g_signal_handlers_disconnect_matched(G_OBJECT(model), G_SIGNAL_MATCH_FUNC, 0, 0, NULL, clue_check_isempty, ui);
@@ -347,7 +349,7 @@ clue_remove_clicked (GtkButton *button, UIData *ui)
 
 	len = gtk_tree_model_iter_n_children (model, NULL);
 	if (len > 0) {
-		if (gtk_list_store_iter_is_valid(GTK_LIST_STORE(model), &iter)) {
+		if (gtk_list_store_iter_is_valid (GTK_LIST_STORE(model), &iter)) {
 			gtk_tree_selection_select_iter (selection, &iter);
 		} else {
 			if (path && valid) {
@@ -384,7 +386,6 @@ clue_edit_clicked (GtkButton *button, UIData *ui)
 
 	if (path) {
 		gtk_tree_view_set_cursor (GTK_TREE_VIEW (ui->treeview), path, focus_col, TRUE);
-
 		gtk_tree_path_free (path);
 	}
 }
@@ -394,9 +395,9 @@ toggle_cb (GtkWidget *widget, UIData *ui)
 {
 
 	gboolean active = gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (widget));
-	GConfClient *gconf = gconf_client_get_default();
+	ui->gconf = gconf_client_get_default();
 
-   	gconf_client_set_bool (gconf, GCONF_KEY_ATTACHMENT_REMINDER, active, NULL);
+   	gconf_client_set_bool (ui->gconf, GCONF_KEY_ATTACHMENT_REMINDER, active, NULL);
 	gtk_widget_set_sensitive (ui->clue_container, active);
 }
 
@@ -415,16 +416,27 @@ selection_changed (GtkTreeSelection *selection, UIData *ui)
 	}
 }
 
+static void
+destroy_ui_data (gpointer data)
+{
+	UIData *ui = (UIData *) data;
+
+	if (!ui)
+		return;
+
+	g_object_unref (ui->xml);
+	g_object_unref (ui->gconf);
+	g_free (ui);
+}
+
 GtkWidget *
 e_plugin_lib_get_configure_widget (EPlugin *epl)
 {
-	GtkWidget *dialog;
-	GladeXML *xml;
 	GtkCellRenderer *renderer;
 	GtkTreeSelection *selection;
 	GtkTreeIter iter;
 	GConfClient *gconf = gconf_client_get_default();
-	GtkWidget *hbox, *button;
+	GtkWidget *button, *hbox;
 	GSList *clue_list = NULL, *list;
 	GtkTreeModel *model;
 	gboolean enable_ui;
@@ -436,20 +448,17 @@ e_plugin_lib_get_configure_widget (EPlugin *epl)
 	gladefile = g_build_filename (EVOLUTION_PLUGINDIR,
 			"attachment-reminder.glade",
 			NULL);
-	xml = glade_xml_new (gladefile, "reminder_configuration_box", NULL);
+	ui->xml = glade_xml_new (gladefile, "reminder_configuration_box", NULL);
 	g_free (gladefile);
 
 	ui->gconf = gconf_client_get_default ();
 	enable_ui = gconf_client_get_bool (ui->gconf, GCONF_KEY_ATTACHMENT_REMINDER, NULL);
 
-	ui->treeview = glade_xml_get_widget (xml, "clue_treeview");
+	ui->treeview = glade_xml_get_widget (ui->xml, "clue_treeview");
 
-	if (store == NULL)
-		store = gtk_list_store_new (CLUE_N_COLUMNS, G_TYPE_STRING);
-	else
-		gtk_list_store_clear (store);
+	ui->store = gtk_list_store_new (CLUE_N_COLUMNS, G_TYPE_STRING);
 
-	gtk_tree_view_set_model (GTK_TREE_VIEW (ui->treeview), GTK_TREE_MODEL (store));
+	gtk_tree_view_set_model (GTK_TREE_VIEW (ui->treeview), GTK_TREE_MODEL (ui->store));
 
 	renderer = gtk_cell_renderer_text_new ();
 	gtk_tree_view_insert_column_with_attributes (GTK_TREE_VIEW (ui->treeview), -1, _("Keywords"),
@@ -462,14 +471,14 @@ e_plugin_lib_get_configure_widget (EPlugin *epl)
 	g_signal_connect (G_OBJECT (selection), "changed", G_CALLBACK (selection_changed), ui);
 	gtk_tree_view_set_headers_visible (GTK_TREE_VIEW (ui->treeview), TRUE);
 
-	ui->clue_add = glade_xml_get_widget (xml, "clue_add");
+	ui->clue_add = glade_xml_get_widget (ui->xml, "clue_add");
 	g_signal_connect (G_OBJECT (ui->clue_add), "clicked", G_CALLBACK (clue_add_clicked), ui);
 
-	ui->clue_remove = glade_xml_get_widget (xml, "clue_remove");
+	ui->clue_remove = glade_xml_get_widget (ui->xml, "clue_remove");
 	g_signal_connect (G_OBJECT (ui->clue_remove), "clicked", G_CALLBACK (clue_remove_clicked), ui);
 	gtk_widget_set_sensitive (ui->clue_remove, FALSE);
 
-	ui->clue_edit = glade_xml_get_widget (xml, "clue_edit");
+	ui->clue_edit = glade_xml_get_widget (ui->xml, "clue_edit");
 	g_signal_connect (G_OBJECT (ui->clue_edit), "clicked", G_CALLBACK (clue_edit_clicked), ui);
 	gtk_widget_set_sensitive (ui->clue_edit, FALSE);
 
@@ -480,8 +489,8 @@ e_plugin_lib_get_configure_widget (EPlugin *epl)
 	clue_list = gconf_client_get_list ( gconf, GCONF_KEY_ATTACH_REMINDER_CLUES, GCONF_VALUE_STRING, NULL );
 
 	for (list = clue_list; list; list = g_slist_next (list)) {
-		gtk_list_store_append (store, &iter);
-		gtk_list_store_set (store, &iter, CLUE_KEYWORD_COLUMN, list->data, -1);
+		gtk_list_store_append (ui->store, &iter);
+		gtk_list_store_set (ui->store, &iter, CLUE_KEYWORD_COLUMN, list->data, -1);
 	}
 
 	if (clue_list) {
@@ -490,32 +499,24 @@ e_plugin_lib_get_configure_widget (EPlugin *epl)
 	}
 
 	/* Enable / Disable */
-	gconf = gconf_client_get_default ();
-	button = glade_xml_get_widget (xml, "reminder_enable_check");
+	button = glade_xml_get_widget (ui->xml, "reminder_enable_check");
 	gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (button) , enable_ui);
 	g_signal_connect (G_OBJECT (button), "toggled", G_CALLBACK (toggle_cb), ui);
 
 	/* Add the list here */
-	ui->clue_container = glade_xml_get_widget (xml, "clue_container");
+	ui->clue_container = glade_xml_get_widget (ui->xml, "clue_container");
 	gtk_widget_set_sensitive (ui->clue_container, enable_ui);
 
-	hbox = glade_xml_get_widget (xml, "reminder_configuration_box");
+	hbox = gtk_vbox_new (FALSE, 0);
 
-	dialog = gtk_dialog_new_with_buttons (_("Attachment Reminder Preferences"),
-			NULL,
-			GTK_DIALOG_DESTROY_WITH_PARENT,
-			GTK_STOCK_CLOSE,
-			GTK_RESPONSE_NONE,
-			NULL);
+	gtk_box_pack_start (GTK_BOX (hbox), glade_xml_get_widget (ui->xml, "reminder_configuration_box"), FALSE, FALSE, 0);
 
-	g_signal_connect_swapped (dialog, "response", G_CALLBACK (gtk_widget_destroy), dialog);
+	/* to let free data properly on destroy of configuration widget */
+	g_object_set_data_full (G_OBJECT (hbox), "myui-data", ui, destroy_ui_data);
 
-	//gtk_container_add (GTK_CONTAINER (GTK_DIALOG(dialog)->vbox), hbox);
-	//gtk_widget_show_all (dialog);
 	return hbox;
 }
 
-
 /* Configuration in Mail Prefs Page goes here */
 
 GtkWidget *