Updated the service, better memory handling, menu item sorting, triggered by about to show signal, discovered a new bug in dbusmenu

https://bugs.launchpad.net/dbusmenu/+bug/713041

1 files changed, 107 insertions, 62 deletions

diff --git a/src/datetime-service.c b/src/datetime-service.c
index 85fcc2d..736e89e 100644
--- a/src/datetime-service.c
+++ b/src/datetime-service.c
@@ -66,6 +66,7 @@ static DbusmenuMenuitem * tzchange = NULL;
 static GList			* appointments = NULL;
 static ECal				* ecal = NULL;
 static const gchar		* ecal_timezone = NULL;
+static icaltimezone     *tzone;
 
 /* Geoclue trackers */
 static GeoclueMasterClient * geo_master = NULL;
@@ -244,6 +245,41 @@ update_timezone_menu_items(gpointer user_data) {
 	return FALSE;
 }
 
+// Compare function for g_list_sort of ECalComponent objects
+static gint 
+compare_appointment_items (ECalComponent *a,
+                           ECalComponent *b) {
+                                       
+	ECalComponentDateTime datetime_a, datetime_b;
+	struct tm tm_a, tm_b;
+	time_t t_a, t_b;
+	gint retval = 0;
+	
+	ECalComponentVType vtype = e_cal_component_get_vtype (a);
+	if (vtype == E_CAL_COMPONENT_EVENT)
+		e_cal_component_get_dtstart (a, &datetime_a);
+	else
+	    e_cal_component_get_due (a, &datetime_a);
+	tm_a = icaltimetype_to_tm(datetime_a.value);
+	t_a = mktime(&tm_a);
+	
+	vtype = e_cal_component_get_vtype (b);
+	if (vtype == E_CAL_COMPONENT_EVENT)
+		e_cal_component_get_dtstart (b, &datetime_b);
+	else
+	    e_cal_component_get_due (b, &datetime_b);
+	tm_b = icaltimetype_to_tm(datetime_b.value);
+	t_b = mktime(&tm_b);
+
+	// Compare datetime_a and datetime_b, newest first in this sort.
+	if (t_a > t_b) retval = 1;
+	else if (t_a < t_b) retval = -1;
+	
+	e_cal_component_free_datetime (&datetime_a);
+	e_cal_component_free_datetime (&datetime_b);
+	return retval;
+}
+
 /* Populate the menu with todays, next 5 appointments. 
  * we should hook into the ABOUT TO SHOW signal and use that to update the appointments.
  * Experience has shown that caldav's and webcals can be slow to load from eds
@@ -251,55 +287,25 @@ update_timezone_menu_items(gpointer user_data) {
  */
 static gboolean
 update_appointment_menu_items (gpointer user_data) {
+	if (!ecal) return FALSE;
 	// FFR: we should take into account short term timers, for instance
 	// tea timers, pomodoro timers etc... that people may add, this is hinted to in the spec.
 	time_t t1, t2;
-	icaltimezone *tzone;
-	gchar *query, *is, *ie;
+	gchar *query, *is, *ie, *ad;
 	GList *objects = NULL, *l;
-	GError *gerror = NULL;
 	DbusmenuMenuitem * item = NULL;
+	GError *gerror = NULL;
 	gint i;
 	gint width, height;
 	
-	g_debug("Setting up ecal.");
-	if (!ecal)
-		ecal = e_cal_new_system_calendar();
-	
-	if (!ecal) {
-		g_debug("e_cal_new_system_calendar failed");
-		ecal = NULL;
-		return FALSE;
-	}
-	g_debug("Open calendar.");
-	if (!e_cal_open(ecal, FALSE, &gerror) ) {
-		g_debug("e_cal_open: %s\n", gerror->message);
-		g_free(ecal);
-		ecal = NULL;
-		return FALSE;
-	}
-	g_debug("Get calendar timezone.");
-	if (!e_cal_get_timezone(ecal, "UTC", &tzone, &gerror)) {
-		g_debug("failed to get time zone\n");
-		g_free(ecal);
-		ecal = NULL;
-		return FALSE;
-	}
-	
-	/* This timezone represents the timezone of the calendar, this might be different to the current UTC offset.
-	 * this means we'll have some geoclue interaction going on, and possibly the user will be involved in setting
-	 * their location manually, case in point: trains have satellite links which often geoclue to sweden,
-	 * this shouldn't automatically set the location and mess up all the appointments for the user.
-	 */
-	ecal_timezone = icaltimezone_get_tzid(tzone);
-	
 	time(&t1);
 	time(&t2);
 	t2 += (time_t) (7 * 24 * 60 * 60); /* 7 days ahead of now */
 
 	is = isodate_from_time_t(t1);
 	ie = isodate_from_time_t(t2);
-	// FIXME can we put a limit on the number of results? Or if not complete, or is event/todo?
+	
+	// FIXME can we put a limit on the number of results? Or if not complete, or is event/todo? Or sort it by date?
 	query = g_strdup_printf("(occur-in-time-range? (make-time\"%s\") (make-time\"%s\"))", is, ie);
 	g_debug("Getting objects with query: %s", query);
 	if (!e_cal_get_object_list_as_comp(ecal, query, &objects, &gerror)) {
@@ -311,16 +317,21 @@ update_appointment_menu_items (gpointer user_data) {
 	g_debug("Number of objects returned: %d", g_list_length(objects));
 	gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height);
 	if (appointments != NULL) {
+		g_debug("Freeing old appointments");
 		for (l = appointments; l; l = l->next) {
+			g_debug("Freeing old appointment");
 			item =  l->data;
 			// Remove all the existing menu items which are in appointments.
-			dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item));
 			appointments = g_list_remove(appointments, item);
-			g_free(item);
+			dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item));
+			//g_free(item); freeing makes it crash :/ is that a double free from child delete?
+			item = NULL;
 		}
 		appointments = NULL;
 	}
 	
+	// Sort the list see above FIXME regarding queries
+	objects = g_list_sort(objects, (GCompareFunc) compare_appointment_items);
 	i = 0;
 	for (l = objects; l; l = l->next) {
 		ECalComponent *ecalcomp = l->data;
@@ -330,7 +341,7 @@ update_appointment_menu_items (gpointer user_data) {
 		icalproperty_status status;
 		gchar *summary, *cmd;
 		char right[20];
-		const gchar *uri;
+		//const gchar *uri;
 		struct tm tmp_tm;
 
 		ECalComponentVType vtype = e_cal_component_get_vtype (ecalcomp);
@@ -346,8 +357,6 @@ update_appointment_menu_items (gpointer user_data) {
 		if (status == ICAL_STATUS_COMPLETED || status == ICAL_STATUS_CANCELLED)
 			continue;
 
-		// INPROGRESS: Create a menu item for each of them, try to include helpful metadata e.g. colours, due time
-
 		item = dbusmenu_menuitem_new();
 		dbusmenu_menuitem_property_set       (item, DBUSMENU_MENUITEM_PROP_TYPE, APPOINTMENT_MENUITEM_TYPE);
 		dbusmenu_menuitem_property_set_bool  (item, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE);
@@ -370,23 +379,23 @@ update_appointment_menu_items (gpointer user_data) {
 		
 		// FIXME need to get the timezone of the above datetime, 
 		// and get the icaltimezone of the geoclue timezone/selected timezone (whichever is preferred)
-		
-		//g_debug("Check for a datetime."); // Should always have one
 		if (!datetime.value) {
 			g_free(item);
 			continue;
 		}
 		
-		//g_debug("Set appointment zone."); 
 		if (!appointment_zone || datetime.value->is_date) { // If it's today put in the current timezone?
 			appointment_zone = tzone;
 		}
 		tmp_tm = icaltimetype_to_tm_with_zone (datetime.value, appointment_zone, tzone);
 		
-		
+		g_debug("Generate time string");
 		// Get today
-		g_debug("Generate strings");
-		if (datetime.value->is_date) // Is today - not working :/
+		time_t curtime = time(NULL);
+  		struct tm* today = localtime(&curtime);
+		if (today->tm_mday == tmp_tm.tm_mday && 
+		    today->tm_mon == tmp_tm.tm_mon && 
+		    today->tm_year == tmp_tm.tm_year)
 			strftime(right, 20, "%l:%M %P", &tmp_tm);
 		else
 			strftime(right, 20, "%a %l:%M %P", &tmp_tm);
@@ -394,12 +403,15 @@ update_appointment_menu_items (gpointer user_data) {
 		g_debug("Appointment time: %s", right);
 		dbusmenu_menuitem_property_set (item, APPOINTMENT_MENUITEM_PROP_RIGHT, right);
 		
-		//g_debug("Free datetime.");
 		e_cal_component_free_datetime (&datetime);
 		
+		ad = is = isodate_from_time_t(mktime(&tmp_tm));
+		
 		// Now we pull out the URI for the calendar event and try to create a URI that'll work when we execute evolution
-		e_cal_component_get_uid(ecalcomp, &uri);
-		cmd = g_strconcat("evolution calendar://", uri, NULL);
+		// FIXME Because the URI stuff is really broken, we're going to open the calendar at todays date instead
+		
+		//e_cal_component_get_uid(ecalcomp, &uri);
+		cmd = g_strconcat("evolution calendar:///?startdate=", ad, NULL);
 		g_signal_connect (G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
 						  G_CALLBACK (activate_cb), cmd);
 		
@@ -407,33 +419,34 @@ update_appointment_menu_items (gpointer user_data) {
 		
 		// Get the colour E_CAL_COMPONENT_FIELD_COLOR
 		// Get the icon, either EVENT or MEMO or TODO?
-		/*gdouble red, blue, green;
-        ECalSource *source = e_cal_get_source (ecalcomp->client);
-        if (!ecalcomp->color && e_source_get_color (source, &source_color)) {
-			g_free (comp_data->color);
-			ecalcomp->color = g_strdup_printf ("#%06x", source_color & 0xffffff);
-        }*/
-        
+		// needs EDS, ecal component colours are broken...
+		//gdouble red, blue, green;
+        //ECalSource *source = e_cal_get_source (ecalcomp->client);
+        //if (!ecalcomp->color && e_source_get_color (source, &source_color)) {
+			//g_free (comp_data->color);
+			//ecalcomp->color = g_strdup_printf ("#%06x", source_color & 0xffffff);
+        //}
+        //g_debug("Colour to use: %s", ecalcomp->color);
              
 		//cairo_surface_t *cs = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, width, height);
 		//cairo_t *cr = cairo_create(cs);
 
 		// TODO: Draw the correct icon for the appointment type and then tint it using mask fill.
-
+		// For now we'll create a circle
+		
+		
 		//GdkPixbuf * pixbuf = gdk_pixbuf_get_from_drawable(NULL, (GdkDrawable*)cs, 0,0,0,0, width, height);
 		
 		//dbusmenu_menuitem_property_set_image (item, APPOINTMENT_MENUITEM_PROP_ICON, pixbuf);
 		
-		dbusmenu_menuitem_child_append       (root, item);
+		dbusmenu_menuitem_child_add_position (root, item, 4+i);
 		appointments = g_list_append         (appointments, item); // Keep track of the items here to make them east to remove
 		
 		if (i == 4) break; // See above FIXME regarding query result limit
 		i++;
 	}
+	g_list_free(objects);
 	g_debug("End of objects");
-	if (ecal) {
-		//g_free(ecal); // Really we should do the setup somewhere where we know it'll only run once, right now, we'll do it every time and free it.
-	}
 	return TRUE;
 }
 
@@ -492,8 +505,39 @@ build_menus (DbusmenuMenuitem * root)
 
 	// This just populates the items on startup later we want to be able to update the appointments before
 	// presenting the menu. 
-	update_appointment_menu_items(NULL);
 	if (calendar != NULL) {
+		GError *gerror = NULL;
+		// TODO: In reality we should iterate sources of calendar, but getting the local one doens't lag for > a minute
+		g_debug("Setting up ecal.");
+		if (!ecal)
+			ecal = e_cal_new_system_calendar();
+	
+		if (!ecal) {
+			g_debug("e_cal_new_system_calendar failed");
+			ecal = NULL;
+		}
+		g_debug("Open calendar.");
+		if (!e_cal_open(ecal, FALSE, &gerror) ) {
+			g_debug("e_cal_open: %s\n", gerror->message);
+			g_free(ecal);
+			ecal = NULL;
+		}
+		g_debug("Get calendar timezone.");
+		if (!e_cal_get_timezone(ecal, "UTC", &tzone, &gerror)) {
+			g_debug("failed to get time zone\n");
+			g_free(ecal);
+			ecal = NULL;
+		}
+	
+		/* This timezone represents the timezone of the calendar, this might be different to the current UTC offset.
+		 * this means we'll have some geoclue interaction going on, and possibly the user will be involved in setting
+		 * their location manually, case in point: trains have satellite links which often geoclue to sweden,
+		 * this shouldn't automatically set the location and mess up all the appointments for the user.
+		 */
+		if (ecal) ecal_timezone = icaltimezone_get_tzid(tzone);
+	
+		g_signal_connect(root, DBUSMENU_MENUITEM_SIGNAL_ABOUT_TO_SHOW, G_CALLBACK(update_appointment_menu_items), NULL);
+		update_appointment_menu_items(NULL);
 		// TODO Create "Add appointment" menu item
 	}
 	// TODO Create FFR? "Add timer" menu item
@@ -751,6 +795,7 @@ main (int argc, char ** argv)
 	server = dbusmenu_server_new(MENU_OBJ);
 	root = dbusmenu_menuitem_new();
 	dbusmenu_server_set_root(server, root);
+	
 	build_menus(root);
 
 	/* Setup geoclue */