Merge lp:~ted/indicator-applet/dynamic-allocation into lp:indicator-applet/0.4

[Ted Gould]

[Ted Gould (ted)]

This patch makes it so that the indicator applet can respond to entries being added and removed.

[Robert Collins (lifeless)]

On Wed, 2009-11-25 at 03:35 +0000, Ted Gould wrote:
>
> + if (entry->image != NULL) {
> + gtk_box_pack_start(GTK_BOX(hbox),
> GTK_WIDGET(entry->image), FALSE, FALSE, 0);
> + }
> + if (entry->label != NULL) {
> + gtk_box_pack_start(GTK_BOX(hbox),
> GTK_WIDGET(entry->label), FALSE, FALSE, 0);
> + }

I'd be tempted to have a helper function for this idiom, though its
right on the lower bound of being useful for that.

Is this something that can be tested?

 review; approve

[David Barth (dbarth)]

review: Approve

[Ted Gould (ted)]

On Wed, 2009-11-25 at 05:36 +0000, Robert Collins wrote:
> On Wed, 2009-11-25 at 03:35 +0000, Ted Gould wrote:
> >
> > + if (entry->image != NULL) {
> > + gtk_box_pack_start(GTK_BOX(hbox),
> > GTK_WIDGET(entry->image), FALSE, FALSE, 0);
> > + }
> > + if (entry->label != NULL) {
> > + gtk_box_pack_start(GTK_BOX(hbox),
> > GTK_WIDGET(entry->label), FALSE, FALSE, 0);
> > + }
>
> I'd be tempted to have a helper function for this idiom, though its
> right on the lower bound of being useful for that.

The problem is that C doesn't have an easy to use "foreach" type
construct. So you end up building datatypes to make it reasonable,
which doesn't make the code any shorter :(

> Is this something that can be tested?

Not really. We'd need a Bonobo test harness. I'm not up for writing
one, especially with Bonobo being deprecated.

  --Ted

[Robert Collins (lifeless)]

On Wed, 2009-11-25 at 21:27 +0000, Ted Gould wrote:
> + if (entry->image != NULL) {
> > > + gtk_box_pack_start(GTK_BOX(hbox),
> > > GTK_WIDGET(entry->image), FALSE, FALSE, 0);

re: datatypes - you have those, don't you? (GTK_WIDGET, GTK_BOX, ..) Or
does GTK_WIDGET barf on NULL ? (in which case
GTK_WIDGET_NULL(entry->image);)

-Rob

[Ted Gould (ted)]

On Wed, 2009-11-25 at 21:51 +0000, Robert Collins wrote:
> On Wed, 2009-11-25 at 21:27 +0000, Ted Gould wrote:
> > + if (entry->image != NULL) {
> > > > + gtk_box_pack_start(GTK_BOX(hbox),
> > > > GTK_WIDGET(entry->image), FALSE, FALSE, 0);
>
> re: datatypes - you have those, don't you? (GTK_WIDGET, GTK_BOX, ..) Or
> does GTK_WIDGET barf on NULL ? (in which case
> GTK_WIDGET_NULL(entry->image);)

I don't believe that GTK_WIDGET will barf. I believe it just returns
NULL. Which (I haven't looked, but I'm pretty confident in) will cause
gtk_box_pack_start() to throw a warning/error. Which we can't catch and
not print because, well, it's C :)

  --Ted

[Robert Collins (lifeless)]

On Wed, 2009-11-25 at 21:57 +0000, Ted Gould wrote:
>
>
> I don't believe that GTK_WIDGET will barf. I believe it just returns
> NULL. Which (I haven't looked, but I'm pretty confident in) will
> cause
> gtk_box_pack_start() to throw a warning/error. Which we can't catch
> and
> not print because, well, it's C :)

I know :)

The helper I had in mind was:

void
foo(GtkWidget *widget ....) {
if (NULL == widget)
   return;
....
}

Note that I'm not asking you to do this, as I mentioned its just
something I'd consider doing.

-Rob

=== modified file 'src/applet-main.c'
--- src/applet-main.c	2009-11-04 20:11:04 +0000
+++ src/applet-main.c	2009-11-25 03:35:21 +0000
@@ -25,6 +25,8 @@
 
 #include "libindicator/indicator-object.h"
 
+#define ENTRY_DATA_NAME "indicator-custom-entry-data"
+
 static gboolean     applet_fill_cb (PanelApplet * applet, const gchar * iid, gpointer data);
 
 
@@ -46,6 +48,58 @@
 /*************
  * init function
  * ***********/
+static void
+entry_added (IndicatorObject * io, IndicatorObjectEntry * entry, GtkWidget * menu)
+{
+	g_debug("Signal: Entry Added");
+
+	GtkWidget * menuitem = gtk_menu_item_new();
+	GtkWidget * hbox = gtk_hbox_new(FALSE, 3);
+
+	if (entry->image != NULL) {
+		gtk_box_pack_start(GTK_BOX(hbox), GTK_WIDGET(entry->image), FALSE, FALSE, 0);
+	}
+	if (entry->label != NULL) {
+		gtk_box_pack_start(GTK_BOX(hbox), GTK_WIDGET(entry->label), FALSE, FALSE, 0);
+	}
+	gtk_container_add(GTK_CONTAINER(menuitem), hbox);
+	gtk_widget_show(hbox);
+
+	if (entry->menu != NULL) {
+		gtk_menu_item_set_submenu(GTK_MENU_ITEM(menuitem), GTK_WIDGET(entry->menu));
+	}
+
+	gtk_menu_shell_append(GTK_MENU_SHELL(menu), menuitem);
+	gtk_widget_show(menuitem);
+
+	g_object_set_data(G_OBJECT(menuitem), ENTRY_DATA_NAME, entry);
+
+	return;
+}
+
+static void
+entry_removed_cb (GtkWidget * widget, gpointer userdata)
+{
+	gpointer data = g_object_get_data(G_OBJECT(widget), ENTRY_DATA_NAME);
+
+	if (data != userdata) {
+		return;
+	}
+
+	gtk_widget_destroy(widget);
+	return;
+}
+
+static void 
+entry_removed (IndicatorObject * io, IndicatorObjectEntry * entry, gpointer user_data)
+{
+	g_debug("Signal: Entry Removed");
+
+	gtk_container_foreach(GTK_CONTAINER(user_data), entry_removed_cb, entry);
+
+	return;
+}
+
 static gboolean
 load_module (const gchar * name, GtkWidget * menu)
 {
@@ -58,33 +112,22 @@
 
 	g_debug("Loading Module: %s", name);
 
+	/* Build the object for the module */
 	gchar * fullpath = g_build_filename(INDICATOR_DIR, name, NULL);
 	IndicatorObject * io = indicator_object_new_from_file(fullpath);
 	g_free(fullpath);
 
+	/* Connect to it's signals */
+	g_signal_connect(G_OBJECT(io), INDICATOR_OBJECT_SIGNAL_ENTRY_ADDED,   G_CALLBACK(entry_added),    menu);
+	g_signal_connect(G_OBJECT(io), INDICATOR_OBJECT_SIGNAL_ENTRY_REMOVED, G_CALLBACK(entry_removed),  menu);
+
+	/* Work on the entries */
 	GList * entries = indicator_object_get_entries(io);
 	GList * entry = NULL;
 
 	for (entry = entries; entry != NULL; entry = g_list_next(entry)) {
 		IndicatorObjectEntry * entrydata = (IndicatorObjectEntry *)entry->data;
-
-		GtkWidget * menuitem = gtk_menu_item_new();
-		GtkWidget * hbox = gtk_hbox_new(FALSE, 3);
-		if (entrydata->image != NULL) {
-			gtk_box_pack_start(GTK_BOX(hbox), GTK_WIDGET(entrydata->image), FALSE, FALSE, 0);
-		}
-		if (entrydata->label != NULL) {
-			gtk_box_pack_start(GTK_BOX(hbox), GTK_WIDGET(entrydata->label), FALSE, FALSE, 0);
-		}
-		gtk_container_add(GTK_CONTAINER(menuitem), hbox);
-		gtk_widget_show(hbox);
-
-		if (entrydata->menu != NULL) {
-			gtk_menu_item_set_submenu(GTK_MENU_ITEM(menuitem), GTK_WIDGET(entrydata->menu));
-		}
-
-		gtk_menu_shell_append(GTK_MENU_SHELL(menu), menuitem);
-		gtk_widget_show(menuitem);
+		entry_added(io, entrydata, menu);
 	}
 
 	g_list_free(entries);