completely rewritten. Simpler and more robust.

2005-02-17  Not Zed  <NotZed@Ximian.com>

	* e-popup.c (ep_prune_tree, ep_build_tree, e_popup_create_menu):
	completely rewritten.  Simpler and more robust.

2005-02-16  Not Zed  <NotZed@Ximian.com>

	* e-popup.c (e_popup_create_menu): do the visibility mask test
	before sorting.  don't perform it on bar's or submenu's anymore,
	calculate when not to show/create them.

svn path=/trunk/; revision=28813

2 files changed, 180 insertions, 83 deletions

diff --git a/e-util/ChangeLog b/e-util/ChangeLog
index f4a0eed16c..65ef2ba7b8 100644
--- a/e-util/ChangeLog
+++ b/e-util/ChangeLog
@@ -1,3 +1,14 @@
+2005-02-17  Not Zed  <NotZed@Ximian.com>
+
+	* e-popup.c (ep_prune_tree, ep_build_tree, e_popup_create_menu):
+	completely rewritten.  Simpler and more robust.
+
+2005-02-16  Not Zed  <NotZed@Ximian.com>
+
+	* e-popup.c (e_popup_create_menu): do the visibility mask test
+	before sorting.  don't perform it on bar's or submenu's anymore,
+	calculate when not to show/create them.
+
 2005-02-14  Rodney Dawes  <dobey@novell.com>
 
 	* e-menu.c (emph_construct): If the plug-in is not enabled, do nothing
diff --git a/e-util/e-popup.c b/e-util/e-popup.c
index 14bbfc6e9b..cb505e0cb2 100644
--- a/e-util/e-popup.c
+++ b/e-util/e-popup.c
@@ -56,7 +56,12 @@ struct _EPopupFactory {
 
 /* Used for the "activate" signal callback data to re-map to the api */
 struct _item_node {
-	struct _item_node *next;
+	struct _item_node *next; /* tree pointers */
+	struct _item_node *prev;
+	struct _item_node *parent;
+	EDList children;
+
+	struct _item_node *link; /* for freeing */
 
 	EPopupItem *item;
 	struct _menu_node *menu;
@@ -113,7 +118,7 @@ ep_finalise(GObject *o)
 		/* free item activate callback data */
 		inode = mnode->items;
 		while (inode) {
-			struct _item_node *nnode = inode->next;
+			struct _item_node *nnode = inode->link;
 
 			g_free(inode);
 			inode = nnode;
@@ -298,87 +303,62 @@ ep_activate(GtkWidget *w, struct _item_node *inode)
 	item->activate(inode->menu->popup, item, inode->menu->data);
 }
 
-/**
- * e_popup_create:
- * @emp: An EPopup derived object.
- * @target: popup target, if set, then factories will be invoked.
- * This is then owned by the menu.
- * @mask: If supplied, overrides the target specified mask or provides
- * a mask if no target is supplied.  Used to enable or show menu
- * items.
- *
- * All of the menu items registered on @emp are sorted by path, and
- * then converted into a menu heirarchy.
- *
- *
- * Return value: A GtkMenu which can be popped up when ready.
- **/
-GtkMenu *
-e_popup_create_menu(EPopup *emp, EPopupTarget *target, guint32 mask)
+static void
+ep_prune_tree(EDList *head)
 {
-	struct _EPopupPrivate *p = emp->priv;
-	struct _menu_node *mnode, *nnode;
-	GPtrArray *items = g_ptr_array_new();
-	GSList *l;
-	GString *ppath = g_string_new("");
-	GtkMenu *topmenu;
-	GHashTable *menu_hash = g_hash_table_new(g_str_hash, g_str_equal),
-		*group_hash = g_hash_table_new(g_str_hash, g_str_equal);
-	int i;
-
-	emp->target = target;
-	ep_add_static_items(emp);
+	struct _item_node *inode, *nnode;
 
-	if (target && mask == 0)
-		mask = target->mask;
+	/* need to do two scans, first to find out if the subtree's
+	 * are empty, then to remove any unecessary bars which may
+	 * become unecessary after the first scan */
 
-	/* FIXME: need to override old ones with new names */
-	mnode = (struct _menu_node *)p->menus.head;
-	nnode = mnode->next;
+	inode = (struct _item_node *)head->head;
+	nnode = inode->next;
 	while (nnode) {
-		for (l=mnode->menu; l; l = l->next) {
-			struct _item_node *inode = g_malloc0(sizeof(*inode));
+		struct _EPopupItem *item = inode->item;
 
-			inode->item = l->data;
-			inode->menu = mnode;
-			inode->next = mnode->items;
-			mnode->items = inode;
+		ep_prune_tree(&inode->children);
 
-			g_ptr_array_add(items, inode);
+		if ((item->type & E_POPUP_TYPE_MASK) == E_POPUP_SUBMENU) {
+			if (e_dlist_empty(&inode->children))
+				e_dlist_remove((EDListNode *)inode);
 		}
-		mnode = nnode;
+
+		inode = nnode;
 		nnode = nnode->next;
 	}
 
-	qsort(items->pdata, items->len, sizeof(items->pdata[0]), ep_cmp);
+	inode = (struct _item_node *)head->head;
+	nnode = inode->next;
+	while (nnode) {
+		struct _EPopupItem *item = inode->item;
+
+		if ((item->type & E_POPUP_TYPE_MASK) == E_POPUP_BAR) {
+			if (inode->prev->prev == NULL
+			    || nnode->next == NULL
+			    || (nnode->item->type & E_POPUP_TYPE_MASK) == E_POPUP_BAR)
+				e_dlist_remove((EDListNode *)inode);
+		}
+
+		inode = nnode;
+		nnode = nnode->next;
+	}
+}
+
+static GtkMenu *
+ep_build_tree(struct _item_node *inode, guint32 mask)
+{
+	struct _item_node *nnode;
+	GtkMenu *topmenu;
+	GHashTable *group_hash = g_hash_table_new(g_str_hash, g_str_equal);
 
 	topmenu = (GtkMenu *)gtk_menu_new();
-	for (i=0;i<items->len;i++) {
+
+	nnode = inode->next;
+	while (nnode) {
 		GtkWidget *label;
-		struct _item_node *inode = items->pdata[i];
 		struct _EPopupItem *item = inode->item;
-		GtkMenu *thismenu;
 		GtkMenuItem *menuitem;
-		char *tmp;
-
-		/* for bar's, the mask is exclusive or */
-		if (item->visible) {
-			if ((item->type & E_POPUP_TYPE_MASK) == E_POPUP_BAR) {
-				if ((item->visible & mask) == item->visible)
-					continue;
-			} else if (item->visible & mask)
-				continue;
-		}
-
-		g_string_truncate(ppath, 0);
-		tmp = strrchr(item->path, '/');
-		if (tmp) {
-			g_string_append_len(ppath, item->path, tmp-item->path);
-			thismenu = g_hash_table_lookup(menu_hash, ppath->str);
-			g_assert(thismenu != NULL);
-		} else {
-			thismenu = topmenu;
-		}
 
 		switch (item->type & E_POPUP_TYPE_MASK) {
 		case E_POPUP_ITEM:
@@ -386,11 +366,10 @@ e_popup_create_menu(EPopup *emp, EPopupTarget *target, guint32 mask)
 				GtkWidget *image;
 
 				/* work-around e-icon-factory not doing GTK_STOCK stuff */
-				if (strncmp((char *)item->image, "gtk-", 4) == 0) {
+				if (strncmp((char *)item->image, "gtk-", 4) == 0)
 					image = gtk_image_new_from_stock((char *)item->image, GTK_ICON_SIZE_MENU);
-				} else {
+				else
 					image = e_icon_factory_get_image((char *)item->image, E_ICON_SIZE_MENU);
-				}
 
 				gtk_widget_show(image);
 				menuitem = (GtkMenuItem *)gtk_image_menu_item_new();
@@ -403,25 +382,24 @@ e_popup_create_menu(EPopup *emp, EPopupTarget *target, guint32 mask)
 			menuitem = (GtkMenuItem *)gtk_check_menu_item_new();
 			gtk_check_menu_item_set_active((GtkCheckMenuItem *)menuitem, item->type & E_POPUP_ACTIVE);
 			break;
-		case E_POPUP_RADIO:
-			menuitem = (GtkMenuItem *)gtk_radio_menu_item_new(g_hash_table_lookup(group_hash, ppath->str));
-			/* FIXME: need to strdup the string */
-			g_hash_table_insert(group_hash, ppath->str, gtk_radio_menu_item_get_group((GtkRadioMenuItem *)menuitem));
+		case E_POPUP_RADIO: {
+			char *ppath = inode->parent?inode->parent->item->path:NULL;
+			
+			menuitem = (GtkMenuItem *)gtk_radio_menu_item_new(g_hash_table_lookup(group_hash, ppath));
+			g_hash_table_insert(group_hash, ppath, gtk_radio_menu_item_get_group((GtkRadioMenuItem *)menuitem));
 			gtk_check_menu_item_set_active((GtkCheckMenuItem *)menuitem, item->type & E_POPUP_ACTIVE);
-			break;
+			break; }
 		case E_POPUP_IMAGE:
 			menuitem = (GtkMenuItem *)gtk_image_menu_item_new();
 			gtk_image_menu_item_set_image((GtkImageMenuItem *)menuitem, item->image);
 			break;
 		case E_POPUP_SUBMENU: {
-			GtkMenu *submenu = (GtkMenu *)gtk_menu_new();
+			GtkMenu *submenu = ep_build_tree((struct _item_node *)inode->children.head, mask);
 
-			g_hash_table_insert(menu_hash, item->path, submenu);
 			menuitem = (GtkMenuItem *)gtk_menu_item_new();
 			gtk_menu_item_set_submenu(menuitem, (GtkWidget *)submenu);
 			break; }
 		case E_POPUP_BAR:
-			/* TODO: double-bar, end-bar stuff? */
 			menuitem = (GtkMenuItem *)gtk_separator_menu_item_new();
 			break;
 		default:
@@ -438,22 +416,130 @@ e_popup_create_menu(EPopup *emp, EPopupTarget *target, guint32 mask)
 		if (item->activate)
 			g_signal_connect(menuitem, "activate", G_CALLBACK(ep_activate), inode);
 
-		gtk_menu_shell_append((GtkMenuShell *)thismenu, (GtkWidget *)menuitem);
+		gtk_menu_shell_append((GtkMenuShell *)topmenu, (GtkWidget *)menuitem);
 
 		if (item->enable & mask)
 			gtk_widget_set_sensitive((GtkWidget *)menuitem, FALSE);
 
 		gtk_widget_show((GtkWidget *)menuitem);
+
+		inode = nnode;
+		nnode = nnode->next;
 	}
 
-	g_string_free(ppath, TRUE);
-	g_ptr_array_free(items, TRUE);
-	g_hash_table_destroy(menu_hash);
 	g_hash_table_destroy(group_hash);
 
 	return topmenu;
 }
 
+/**
+ * e_popup_create:
+ * @emp: An EPopup derived object.
+ * @target: popup target, if set, then factories will be invoked.
+ * This is then owned by the menu.
+ * @mask: If supplied, overrides the target specified mask or provides
+ * a mask if no target is supplied.  Used to enable or show menu
+ * items.
+ *
+ * All of the menu items registered on @emp are sorted by path, and
+ * then converted into a menu heirarchy.
+ *
+ *
+ * Return value: A GtkMenu which can be popped up when ready.
+ **/
+GtkMenu *
+e_popup_create_menu(EPopup *emp, EPopupTarget *target, guint32 mask)
+{
+	struct _EPopupPrivate *p = emp->priv;
+	struct _menu_node *mnode, *nnode;
+	GPtrArray *items = g_ptr_array_new();
+	GSList *l;
+	GString *ppath = g_string_new("");
+	GHashTable *tree_hash = g_hash_table_new(g_str_hash, g_str_equal);
+	EDList head = E_DLIST_INITIALISER(head);
+	int i;
+
+	emp->target = target;
+	ep_add_static_items(emp);
+
+	if (target && mask == 0)
+		mask = target->mask;
+
+	/* Note: This code vastly simplifies memory management by
+	 * keeping a linked list of all temporary tree nodes on the
+	 * menu's tree until the epopup is destroyed */
+
+	/* FIXME: need to override old ones with new names */
+	mnode = (struct _menu_node *)p->menus.head;
+	nnode = mnode->next;
+	while (nnode) {
+		for (l=mnode->menu; l; l = l->next) {
+			struct _item_node *inode;
+			struct _EPopupItem *item = l->data;
+
+			/* we calculate bar/submenu visibility based on calculated set */
+			if (item->visible) {
+				if ((item->type & E_POPUP_TYPE_MASK) != E_POPUP_BAR
+				    && (item->type & E_POPUP_TYPE_MASK) != E_POPUP_SUBMENU
+				    && item->visible & mask) {
+					d(printf("%s not visible\n", item->path));
+					continue;
+				}
+			}
+
+			inode = g_malloc0(sizeof(*inode));
+			inode->item = l->data;
+			inode->menu = mnode;
+			e_dlist_init(&inode->children);
+			inode->link = mnode->items;
+			mnode->items = inode;
+
+			g_ptr_array_add(items, inode);
+		}
+		mnode = nnode;
+		nnode = nnode->next;
+	}
+
+	/* this makes building the tree in the right order easier */
+	qsort(items->pdata, items->len, sizeof(items->pdata[0]), ep_cmp);
+
+	/* create tree structure */
+	for (i=0;i<items->len;i++) {
+		struct _item_node *inode = items->pdata[i], *pnode;
+		struct _EPopupItem *item = inode->item;
+		const char *tmp;
+
+		g_string_truncate(ppath, 0);
+		tmp = strrchr(item->path, '/');
+		if (tmp) {
+			g_string_append_len(ppath, item->path, tmp-item->path);
+			pnode = g_hash_table_lookup(tree_hash, ppath->str);
+			if (pnode == NULL) {
+				g_warning("No parent defined for node '%s'", item->path);
+				e_dlist_addtail(&head, (EDListNode *)inode);
+			} else {
+				e_dlist_addtail(&pnode->children, (EDListNode *)inode);
+				inode->parent = pnode;
+			}
+		} else {
+			e_dlist_addtail(&head, (EDListNode *)inode);
+		}
+
+		if ((item->type & E_POPUP_TYPE_MASK) == E_POPUP_SUBMENU)
+			g_hash_table_insert(tree_hash, item->path, inode);
+	}
+
+	g_string_free(ppath, TRUE);
+	g_ptr_array_free(items, TRUE);
+	g_hash_table_destroy(tree_hash);
+
+	/* prune unnecessary items */
+	ep_prune_tree(&head);
+
+	/* & build it */
+	return ep_build_tree((struct _item_node *)head.head, mask);
+}
+
 static void
 ep_popup_done(GtkWidget *w, EPopup *emp)
 {