Re: [patch] attribute editor/wrapper mem cleanup and signal fixes

Index: src/ChangeLog
===================================================================
RCS file: /cvs/gnome/conglomerate/src/ChangeLog,v
retrieving revision 1.736
diff -u -r1.736 ChangeLog
--- src/ChangeLog	15 Oct 2004 16:01:40 -0000	1.736
+++ src/ChangeLog	17 Oct 2004 15:56:48 -0000
@@ -1,3 +1,10 @@
+2004-10-17  Douglas Burke  <[email protected]>
+
+	* cong-attribute-editor.c, cong-attribute-editor-cdata.c, cong-attribute-editor-lang.c, cong-attribute-editor-nmtoken.c, cong-attribute-wrapper.c, cong-attribute-wrapper-check-button.c, cong-attribute-wrapper-radio-button.c:
+
+	Fixed assertion failure from g_signal_handler_block/unblock.
+	Cleanup of memory leaks.
+
 2004-10-15  Douglas Burke  <[email protected]>
 
 	* cong-attribute-editor-cdata.c, cong-attribute-editor-cdata.h, cong-attribute-editor-enumeration.c, cong-attribute-editor-lang.c, cong-attribute-editor-lang.h, cong-attribute-editor-nmtoken.c, cong-attribute-editor-nmtoken.h, cong-attribute-editor.c, cong-attribute-editor.h, cong-attribute-wrapper-check-button.c, cong-attribute-wrapper-check-button.h, cong-attribute-wrapper-radio-button.c, cong-attribute-wrapper-radio-button.h, cong-attribute-wrapper.c, cong-attribute-wrapper.h, cong-glade.c:
Index: src/cong-attribute-editor-cdata.c
===================================================================
RCS file: /cvs/gnome/conglomerate/src/cong-attribute-editor-cdata.c,v
retrieving revision 1.11
diff -u -r1.11 cong-attribute-editor-cdata.c
--- src/cong-attribute-editor-cdata.c	15 Oct 2004 16:01:40 -0000	1.11
+++ src/cong-attribute-editor-cdata.c	17 Oct 2004 15:56:48 -0000
@@ -125,9 +125,6 @@
 			  0);
 	gtk_container_add (GTK_CONTAINER(attribute_editor_cdata),
 			   GTK_WIDGET(PRIVATE(attribute_editor_cdata)->hbox));
-
-	do_refresh (attribute_editor_cdata);
-
 	gtk_widget_show (GTK_WIDGET(PRIVATE(attribute_editor_cdata)->hbox));
 
 	PRIVATE(attribute_editor_cdata)->handler_id_changed = g_signal_connect_after (G_OBJECT(PRIVATE(attribute_editor_cdata)->entry),
@@ -143,6 +140,9 @@
 				G_CALLBACK(on_delete_button),
 				attribute_editor_cdata);
 
+	/* must be called after the signals are created */
+	do_refresh (attribute_editor_cdata);
+
 	return CONG_ATTRIBUTE_EDITOR (attribute_editor_cdata);
 }
 
@@ -252,12 +252,12 @@
 	gchar *attr_value = cong_attribute_editor_get_attribute_value (CONG_ATTRIBUTE_EDITOR(attribute_editor_cdata));
 	
 	if (attr_value) {
-		g_signal_handler_block ( G_OBJECT(PRIVATE(attribute_editor_cdata)->entry),
-					 PRIVATE(attribute_editor_cdata)->handler_id_changed);
+		g_signal_handler_block (G_OBJECT(PRIVATE(attribute_editor_cdata)->entry),
+					PRIVATE(attribute_editor_cdata)->handler_id_changed);
 		gtk_entry_set_text (GTK_ENTRY (PRIVATE(attribute_editor_cdata)->entry),
 				    attr_value);
-		g_signal_handler_unblock ( G_OBJECT(PRIVATE(attribute_editor_cdata)->entry),
-					 PRIVATE(attribute_editor_cdata)->handler_id_changed);
+		g_signal_handler_unblock (G_OBJECT(PRIVATE(attribute_editor_cdata)->entry),
+					  PRIVATE(attribute_editor_cdata)->handler_id_changed);
 
 		g_free (attr_value);
 
Index: src/cong-attribute-editor-lang.c
===================================================================
RCS file: /cvs/gnome/conglomerate/src/cong-attribute-editor-lang.c,v
retrieving revision 1.5
diff -u -r1.5 cong-attribute-editor-lang.c
--- src/cong-attribute-editor-lang.c	15 Oct 2004 16:01:40 -0000	1.5
+++ src/cong-attribute-editor-lang.c	17 Oct 2004 15:56:48 -0000
@@ -261,10 +261,7 @@
         valid = gtk_tree_model_get_iter_first (model, &iter);
  
  	if (attr_value) {
-	              
-	       
 	       /* Skip first row - no language */
-	       
 	       valid = gtk_tree_model_iter_next (model, &iter);
 
 	       while (valid) {
@@ -285,6 +282,8 @@
 		    g_free (value);
 	    	    valid = gtk_tree_model_iter_next (model, &iter);
 	       }
+
+	       g_free (attr_value);
 	} else {
    	       gtk_tree_selection_select_iter (selection, &iter);
 	}
Index: src/cong-attribute-editor-nmtoken.c
===================================================================
RCS file: /cvs/gnome/conglomerate/src/cong-attribute-editor-nmtoken.c,v
retrieving revision 1.2
diff -u -r1.2 cong-attribute-editor-nmtoken.c
--- src/cong-attribute-editor-nmtoken.c	15 Oct 2004 16:01:40 -0000	1.2
+++ src/cong-attribute-editor-nmtoken.c	17 Oct 2004 15:56:48 -0000
@@ -87,7 +87,7 @@
  * @ns_ptr:
  * @attribute_name:
  *
- * Constructror called by #cong_attribute_editor_nmtoken_new()
+ * Constructor called by #cong_attribute_editor_nmtoken_new()
  *
  * Returns:
  */
@@ -112,16 +112,24 @@
 	PRIVATE(attribute_editor_nmtoken)->add_btn = GTK_BUTTON(gtk_button_new_from_stock (GTK_STOCK_ADD));
 	PRIVATE(attribute_editor_nmtoken)->delete_btn = GTK_BUTTON(gtk_button_new_from_stock (GTK_STOCK_DELETE));
 
-	gtk_box_pack_end(PRIVATE(attribute_editor_nmtoken)->hbox, GTK_WIDGET(PRIVATE(attribute_editor_nmtoken)->delete_btn), FALSE, FALSE, 0);
-	gtk_box_pack_end(PRIVATE(attribute_editor_nmtoken)->hbox, GTK_WIDGET(PRIVATE(attribute_editor_nmtoken)->entry), TRUE, TRUE, 0);
-
-	gtk_box_pack_end(PRIVATE(attribute_editor_nmtoken)->hbox, GTK_WIDGET(PRIVATE(attribute_editor_nmtoken)->add_btn), FALSE, FALSE, 0);
+	gtk_box_pack_end (PRIVATE(attribute_editor_nmtoken)->hbox,
+			  GTK_WIDGET(PRIVATE(attribute_editor_nmtoken)->delete_btn),
+			  FALSE,
+			  FALSE,
+			  0);
+	gtk_box_pack_end (PRIVATE(attribute_editor_nmtoken)->hbox,
+			  GTK_WIDGET(PRIVATE(attribute_editor_nmtoken)->entry),
+			  TRUE,
+			  TRUE,
+			  0);
+	gtk_box_pack_end (PRIVATE(attribute_editor_nmtoken)->hbox,
+			  GTK_WIDGET(PRIVATE(attribute_editor_nmtoken)->add_btn),
+			  FALSE,
+			  FALSE,
+			  0);
 
 	gtk_container_add (GTK_CONTAINER(attribute_editor_nmtoken),
 			   GTK_WIDGET(PRIVATE(attribute_editor_nmtoken)->hbox));
-
-	do_refresh (attribute_editor_nmtoken);
-
 	gtk_widget_show (GTK_WIDGET(PRIVATE(attribute_editor_nmtoken)->hbox));
 
 	PRIVATE(attribute_editor_nmtoken)->handler_id_changed = g_signal_connect_after (G_OBJECT(PRIVATE(attribute_editor_nmtoken)->entry),
@@ -137,6 +145,9 @@
 				G_CALLBACK(on_delete_button),
 				attribute_editor_nmtoken);
 
+	/* must be called after the signals are created */
+	do_refresh (attribute_editor_nmtoken);
+
 	return CONG_ATTRIBUTE_EDITOR (attribute_editor_nmtoken);
 }
 
@@ -200,15 +211,24 @@
 		cong_attribute_editor_try_set_value (CONG_ATTRIBUTE_EDITOR(attribute_editor_nmtoken), value);
 
 	} else {
-		/* are there mem leaks here */
+		/*
+		 * Use a dialog to tell the user that the last character they entered
+		 * is not valid. It is rather disruptive, but I am not sure how elde to do
+		 * it. We could change the background colour of the text entry
+		 * to indicate a problem (it avoids the intrusive-ness of the dialog
+		 * and allows a user to delete all the contents and start editing),
+		 * but I'm not sure it's a good piece of UI.
+		 *
+		 * We need to clean up value since it if contains < or > (perhaps others)
+		 * Pango gets all confused. We probably just need to convert to &lt;/&gt;.
+		 */
 		gchar *what_failed = g_strdup_printf (_("Unable to set attribute \"%s\" to \"%s\""),
 						      cong_attribute_editor_get_attribute_name (CONG_ATTRIBUTE_EDITOR(attribute_editor_nmtoken)),
 						      value);
 		CongDocument *doc = cong_attribute_editor_get_document (CONG_ATTRIBUTE_EDITOR(attribute_editor_nmtoken));
 		GtkWindow *parent_window;
 		GtkDialog *dialog;
-
-		const gchar *curr_value;
+		gchar *curr_value;
 
 		parent_window = cong_primary_window_get_toplevel (cong_document_get_primary_window (doc));
 		dialog = cong_error_dialog_new (parent_window,
@@ -220,6 +240,8 @@
 		cong_error_dialog_run (dialog);
 		gtk_widget_destroy (GTK_WIDGET(dialog));
 
+		g_free (what_failed);
+
 		/* 
 		 * restore the previous value: should this be done *before* the dialog is called
 		 *
@@ -233,6 +255,7 @@
 		g_signal_handler_unblock ( G_OBJECT(PRIVATE(attribute_editor_nmtoken)->entry),
 					 PRIVATE(attribute_editor_nmtoken)->handler_id_changed);
 
+		g_free (curr_value);
 		return;
 	}
 }
@@ -295,11 +318,11 @@
 	gchar *attr_value = cong_attribute_editor_get_attribute_value (CONG_ATTRIBUTE_EDITOR(attribute_editor_nmtoken));
 	
 	if (attr_value) {
-		g_signal_handler_block ( G_OBJECT(PRIVATE(attribute_editor_nmtoken)->entry),
-					 PRIVATE(attribute_editor_nmtoken)->handler_id_changed);
+		g_signal_handler_block (G_OBJECT(PRIVATE(attribute_editor_nmtoken)->entry),
+					PRIVATE(attribute_editor_nmtoken)->handler_id_changed);
 		gtk_entry_set_text (GTK_ENTRY (PRIVATE(attribute_editor_nmtoken)->entry),
 				    attr_value);
-		g_signal_handler_unblock ( G_OBJECT(PRIVATE(attribute_editor_nmtoken)->entry),
+		g_signal_handler_unblock (G_OBJECT(PRIVATE(attribute_editor_nmtoken)->entry),
 					 PRIVATE(attribute_editor_nmtoken)->handler_id_changed);
 
 		g_free (attr_value);
Index: src/cong-attribute-editor.c
===================================================================
RCS file: /cvs/gnome/conglomerate/src/cong-attribute-editor.c,v
retrieving revision 1.15
diff -u -r1.15 cong-attribute-editor.c
--- src/cong-attribute-editor.c	15 Oct 2004 16:01:40 -0000	1.15
+++ src/cong-attribute-editor.c	17 Oct 2004 15:56:48 -0000
@@ -201,8 +201,11 @@
  * cong_attribute_editor_get_attribute_value:
  * @attribute_editor:
  *
- * TODO: Write me
- * Returns:
+ * Returns the value of the attribute represented by this editor
+ * (@attribute_editor).
+ *
+ * Returns: The contents of the attribute, to be freed by the called.
+ *          Will be NULL if the attribute is not defined.
  */
 gchar*
 cong_attribute_editor_get_attribute_value (CongAttributeEditor *attribute_editor)
@@ -243,10 +246,12 @@
 		gchar *desc;
 
 		if (new_attr_value) {
-    			desc = g_strdup_printf ( _("Set attribute \"%s\" to \"%s\""), attribute_name, new_attr_value);
-			
+    			desc = g_strdup_printf (_("Set attribute \"%s\" to \"%s\""),
+						attribute_name, 
+						new_attr_value);
 		} else {
-			desc = g_strdup_printf ( _("Delete attribute \"%s\""), attribute_name);
+			desc = g_strdup_printf (_("Delete attribute \"%s\""),
+						attribute_name);
 		}
 
 		cmd = cong_document_begin_command (doc,
@@ -268,6 +273,7 @@
 
 		cong_document_end_command (doc,
 					   cmd);
+		g_free (desc);
 	}
 
 	if (old_attr_value) {
@@ -380,9 +386,12 @@
 	
 		g_signal_handler_disconnect (G_OBJECT (PRIVATE(attribute_editor)->doc),
 					     PRIVATE(attribute_editor)->handler_id_node_set_attribute);
+		PRIVATE(attribute_editor)->handler_id_node_set_attribute = 0;
+
 		g_signal_handler_disconnect (G_OBJECT (PRIVATE(attribute_editor)->doc),
 					     PRIVATE(attribute_editor)->handler_id_node_remove_attribute);
-		
+		PRIVATE(attribute_editor)->handler_id_node_remove_attribute = 0;
+
 		g_object_unref (G_OBJECT (PRIVATE(attribute_editor)->doc));
 		PRIVATE(attribute_editor)->doc = NULL;
 		
Index: src/cong-attribute-wrapper-check-button.c
===================================================================
RCS file: /cvs/gnome/conglomerate/src/cong-attribute-wrapper-check-button.c,v
retrieving revision 1.7
diff -u -r1.7 cong-attribute-wrapper-check-button.c
--- src/cong-attribute-wrapper-check-button.c	15 Oct 2004 16:01:40 -0000	1.7
+++ src/cong-attribute-wrapper-check-button.c	17 Oct 2004 15:56:48 -0000
@@ -237,19 +237,14 @@
 should_button_be_active (CongAttributeWrapperCheckButton *attribute_wrapper)
 {
 	gchar *attribute_value = cong_attribute_wrapper_get_attribute_value (CONG_ATTRIBUTE_WRAPPER(attribute_wrapper));
+	gboolean retval = FALSE;
 
 	if (attribute_value) {
-
 		if (0==strcmp(attribute_value, 
 			      PRIVATE(attribute_wrapper)->attribute_value_checked)) {
-			
-			g_free (attribute_value);
-			return TRUE;			
-		} else {
-			g_free (attribute_value);
-			return FALSE;
+			retval = TRUE;
 		}
-	} else {
-		return FALSE;
+		g_free (attribute_value);
 	}
+	return retval;
 }
Index: src/cong-attribute-wrapper-radio-button.c
===================================================================
RCS file: /cvs/gnome/conglomerate/src/cong-attribute-wrapper-radio-button.c,v
retrieving revision 1.7
diff -u -r1.7 cong-attribute-wrapper-radio-button.c
--- src/cong-attribute-wrapper-radio-button.c	15 Oct 2004 16:01:40 -0000	1.7
+++ src/cong-attribute-wrapper-radio-button.c	17 Oct 2004 15:56:48 -0000
@@ -223,19 +223,18 @@
 should_button_be_active (CongAttributeWrapperRadioButton *attribute_wrapper)
 {
 	gchar *attribute_value = cong_attribute_wrapper_get_attribute_value (CONG_ATTRIBUTE_WRAPPER(attribute_wrapper));
-
+	gboolean retval;
 	if (attribute_value) {
 
 		if (0==strcmp(attribute_value, 
 			      PRIVATE(attribute_wrapper)->attribute_value)) {
-			
-			g_free (attribute_value);
-			return TRUE;			
+			retval = TRUE;
 		} else {
-			g_free (attribute_value);
-			return FALSE;
+			retval = FALSE;
 		}
+		g_free (attribute_value);
 	} else {
-		return (0==strcmp("", PRIVATE(attribute_wrapper)->attribute_value));
+		retval = (0==strcmp("", PRIVATE(attribute_wrapper)->attribute_value));
 	}
+	return retval;
 }
Index: src/cong-attribute-wrapper.c
===================================================================
RCS file: /cvs/gnome/conglomerate/src/cong-attribute-wrapper.c,v
retrieving revision 1.11
diff -u -r1.11 cong-attribute-wrapper.c
--- src/cong-attribute-wrapper.c	15 Oct 2004 16:01:40 -0000	1.11
+++ src/cong-attribute-wrapper.c	17 Oct 2004 15:56:48 -0000
@@ -197,9 +197,12 @@
  * cong_attribute_wrapper_get_attribute_value:
  * @attribute_wrapper:
  *
- * TODO: Write me
- * Returns:
+ * Returns the content of the attribute represented by @attribute_wrapper.
+ *
+ * Returns: The content of the attribute, to be freed by the caller.
+ *          Can be NULL.
  */
+
 gchar*
 cong_attribute_wrapper_get_attribute_value (CongAttributeWrapper *attribute_wrapper)
 {