Merge lp:~mterry/indicator-appmenu/cleaner-remove into lp:indicator-appmenu/0.3

[Michael Terry]

So when I was in the indicator-appmenu code, I noticed a TODO in the bit that removes entries from the window menu code. It currently just deleted the last entry in the list instead of the requested entry.

I haven't found code that hits this bug, so I'm not sure how important this is, but since I was there, I whipped up a little searcher function and used it there and elsewhere to grab an entry from the internal list.

Didn't seem to make things worse, though it also didn't make things noticeably better.

[Ted Gould (ted)]

Ah, I couldn't figure out why I hadn't done the search before with the entries' menu item pointer. Forgot that I added that recently :) Good fix!

review: Approve

=== modified file 'src/window-menus.c'
--- src/window-menus.c	2011-01-26 22:21:20 +0000
+++ src/window-menus.c	2011-02-02 19:14:52 +0000
@@ -338,6 +338,26 @@
 	return;
 }
 
+static IndicatorObjectEntry *
+get_entry(WindowMenus *wm, DbusmenuMenuitem * item, guint *index)
+{
+	WindowMenusPrivate * priv = WINDOW_MENUS_GET_PRIVATE(wm);
+
+	guint position = 0;
+	for (position = 0; position < priv->entries->len; ++position) {
+		WMEntry * entry = g_array_index(priv->entries, WMEntry *, position);
+		if (entry->mi == item) {
+			if (index != NULL) {
+				*index = position;
+			}
+			return &entry->ioentry;
+		}
+	}
+
+	/* Not found */
+	return NULL;
+}
+
 /* Called when a menu item wants to be displayed.  We need to see if
    it's one of our root items and pass it up if so. */
 static void
@@ -350,33 +370,12 @@
 		return;
 	}
 
-	GList * children = dbusmenu_menuitem_get_children(priv->root);
-	guint position = 0;
-	GList * child;
-
-	for (child = children; child != NULL; position++, child = g_list_next(child)) {
-		DbusmenuMenuitem * childmi = DBUSMENU_MENUITEM(child->data);
-
-		/* We're only putting items with children on the panel, so
-		   they're the only one with entires. */
-		if (dbusmenu_menuitem_get_children(childmi) == NULL) {
-			position--;
-			continue;
-		}
-
-		if (childmi == item) {
-			break;
-		}
-	}
-
-	/* Not found */
-	if (child == NULL) {
+	IndicatorObjectEntry * entry = get_entry(WINDOW_MENUS(user_data), item, NULL);
+	if (entry == NULL) {
+		/* Not found */
 		return;
 	}
 
-	g_return_if_fail(position < priv->entries->len);
-
-	IndicatorObjectEntry * entry = g_array_index(priv->entries, IndicatorObjectEntry *, position);
 	g_signal_emit(G_OBJECT(user_data), signals[SHOW_MENU], 0, entry, timestamp, TRUE);
 
 	return;
@@ -715,9 +714,14 @@
 		return;
 	}
 	
-	/* TODO: find the menuitem */
-	IndicatorObjectEntry * entry = g_array_index(priv->entries, IndicatorObjectEntry *, priv->entries->len - 1);
-	g_array_remove_index(priv->entries, priv->entries->len - 1);
+	guint position;
+	IndicatorObjectEntry * entry = get_entry(WINDOW_MENUS(user_data), oldentry, &position);
+	if (entry == NULL) {
+		/* Not found */
+		return;
+	}
+
+	g_array_remove_index(priv->entries, position);
 
 	g_signal_emit(G_OBJECT(user_data), signals[ENTRY_REMOVED], 0, entry, TRUE);