Honestly, I don't see anything odd. I'm going to have to look closer tomorrow. Especially nothing that would cause the craziness that you're getting! Uhg...
On Wed, 2011-01-26 at 11:50 +0000, Karl Lattimer wrote: > Karl Lattimer has proposed merging > lp:~karl-qdh/ubuntu/natty/indicator-datetime/indicator-datetime.withappointments > into lp:indicator-datetime. > > Requested reviews: > Ted Gould (ted) > Related bugs: > #542218 integration with Evolution calendar events and time locations > https://bugs.launchpad.net/bugs/542218 > > For more details, see: > https://code.launchpad.net/~karl-qdh/ubuntu/natty/indicator-datetime/indicator-datetime.withappointments/+merge/47514 > > This change introduces a new dbusmenu item type of APPOINTMENT_MENUITEM_TYPE > which has 3 columns of <image><appointment summary> -- <due time/date> > > with the new item type, we also introduce a small part of code to > datetime-service which reads the EDS data for ecal and produces the values to > fill into the menu items. > > However, there are issues with this. > > For some bizarre reason this indicator is affecting other indicators. Most > notably the sound indicator which switches the volume up/down repeatedly, > other indicators have strange effects too and it's difficult to see why this > can happen in the code itself. > differences between files attachment (review-diff.txt) > === modified file 'configure.ac' > --- configure.ac 2011-01-11 16:19:07 +0000 > +++ configure.ac 2011-01-26 11:50:11 +0000 > @@ -59,6 +59,8 @@ > INDICATOR_DISPLAY_OBJECTS=0.1.10 > GEOCLUE_REQUIRED_VERSION=0.12.0 > OOBS_REQUIRED_VERSION=2.31.0 > +ECAL_REQUIRED_VERSION=2.30 > +ICAL_REQUIRED_VERSION=0.44 > > AS_IF([test "x$with_gtk" = x3], > [PKG_CHECK_MODULES(INDICATOR, indicator3 >= $INDICATOR_REQUIRED_VERSION > @@ -80,7 +82,9 @@ > libido-0.1 >= $INDICATOR_DISPLAY_OBJECTS > gio-2.0 >= $GIO_REQUIRED_VERSION > geoclue >= $GEOCLUE_REQUIRED_VERSION > - liboobs-1 >= $OOBS_REQUIRED_VERSION) > + liboobs-1 >= $OOBS_REQUIRED_VERSION > + libecal-1.2 >= $ECAL_REQUIRED_VERSION > + libical >= $ICAL_REQUIRED_VERSION) > > AC_SUBST(INDICATOR_CFLAGS) > AC_SUBST(INDICATOR_LIBS) > > === modified file 'src/datetime-service.c' > --- src/datetime-service.c 2010-10-22 13:31:37 +0000 > +++ src/datetime-service.c 2011-01-26 11:50:11 +0000 > @@ -23,6 +23,7 @@ > #include <libindicator/indicator-service.h> > #include <locale.h> > > +#include <gtk/gtk.h> > #include <glib/gi18n.h> > #include <gio/gio.h> > > @@ -33,6 +34,15 @@ > #include <geoclue/geoclue-master.h> > #include <geoclue/geoclue-master-client.h> > > +#include <time.h> > +#include <libecal/e-cal.h> > +#include <libical/ical.h> > +#include <libecal/e-cal-time-util.h> > +#include <libedataserver/e-source.h> > +// Other users of ecal seem to also include these, not sure why they should > be included by the above > +#include <libical/icaltime.h> > + > + > #include <oobs/oobs-timeconfig.h> > > #include "datetime-interface.h" > @@ -53,6 +63,9 @@ > static DbusmenuMenuitem * calendar = NULL; > static DbusmenuMenuitem * settings = NULL; > static DbusmenuMenuitem * tzchange = NULL; > +static GList * appointments = NULL; > +static ECal * ecal = NULL; > +static const gchar * ecal_timezone = NULL; > > /* Geoclue trackers */ > static GeoclueMasterClient * geo_master = NULL; > @@ -225,6 +238,186 @@ > return FALSE; > } > > +static gboolean > +update_timezone_menu_items(gpointer user_data) { > + // Get the location preferences and the current location, highlight the > current location somehow > + return FALSE; > +} > + > +/* 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 > + * this is a problem mainly on the EDS side of things, not ours. > + */ > +static gboolean > +update_appointment_menu_items (gpointer user_data) { > + // 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; > + GList *objects = NULL, *l; > + GError *gerror = NULL; > + DbusmenuMenuitem * item = NULL; > + gint i; > + gint width, height; > + > + if (!ecal) > + ecal = e_cal_new_system_calendar(); > + > + if (!ecal) { > + g_debug("e_cal_new_system_calendar failed"); > + ecal = NULL; > + return FALSE; > + } > + if (!e_cal_open(ecal, FALSE, &gerror) ) { > + g_debug("e_cal_open: %s\n", gerror->message); > + g_free(ecal); > + ecal = NULL; > + return FALSE; > + } > + > + 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? > + query = g_strdup_printf("(occur-in-time-range? (make-time\"%s\") > (make-time\"%s\"))", is, ie); > + > + if (!e_cal_get_object_list_as_comp(ecal, query, &objects, &gerror)) { > + g_debug("Failed to get objects\n"); > + g_free(ecal); > + ecal = NULL; > + return FALSE; > + } > + gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height); > + if (appointments != NULL) { > + for (l = appointments; l; l = l->next) { > + 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); > + } > + appointments = NULL; > + } > + > + i = 0; > + for (l = objects; l; l = l->next) { > + ECalComponent *ecalcomp = l->data; > + ECalComponentText valuetext; > + ECalComponentDateTime datetime; > + icaltimezone *appointment_zone = NULL; > + icalproperty_status status; > + gchar *summary, right[20], *cmd; > + const gchar *uri; > + struct tm tmp_tm; > + > + ECalComponentVType vtype = e_cal_component_get_vtype (ecalcomp); > + > + // See above FIXME regarding query result > + // If it's not an event or todo, continue no-increment > + if (vtype != E_CAL_COMPONENT_EVENT || vtype != E_CAL_COMPONENT_TODO) > + continue; > + > + // See above FIXME regarding query result > + // if the status is completed, continue no-increment > + e_cal_component_get_status (ecalcomp, &status); > + 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, "type", > APPOINTMENT_MENUITEM_TYPE); > + dbusmenu_menuitem_property_set_bool (item, > DBUSMENU_MENUITEM_PROP_ENABLED, TRUE); > + dbusmenu_menuitem_property_set_bool (item, > DBUSMENU_MENUITEM_PROP_VISIBLE, TRUE); > + > + // Label text > + e_cal_component_get_summary (ecalcomp, &valuetext); > + summary = g_strdup (valuetext.value); > + > + dbusmenu_menuitem_property_set (item, > APPOINTMENT_MENUITEM_PROP_LABEL, summary); > + > + g_free (summary); > + > + // Due text > + if (vtype == E_CAL_COMPONENT_EVENT) > + e_cal_component_get_dtstart (ecalcomp, &datetime); > + else > + e_cal_component_get_due (ecalcomp, &datetime); > + > + // FIXME need to get the timezone of the above datetime, > + // and get the icaltimezone of the geoclue timezone/selected > timezone (whichever is preferred) > + if (!datetime.value) { > + g_free(item); > + continue; > + } > + > + 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); > + > + e_cal_component_free_datetime (&datetime); > + > + // Get today > + if (datetime.value->is_date) > + strftime(right, sizeof(right), "%X", &tmp_tm); > + else > + strftime(right, sizeof(right), "%a %X", &tmp_tm); > + dbusmenu_menuitem_property_set (item, > APPOINTMENT_MENUITEM_PROP_RIGHT, right); > + > + // 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_url(ecalcomp, &uri); > + cmd = g_strconcat("evolution ", uri, NULL); > + g_signal_connect (G_OBJECT(item), > DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, > + G_CALLBACK (activate_cb), > cmd); > + > + // 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); > + }*/ > + > + > + //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. > + > + //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); > + 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_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; > +} > + > /* Looks for the time and date admin application and enables the > item we have one */ > static gboolean > @@ -272,8 +465,28 @@ > > g_idle_add(check_for_calendar, NULL); > } > + DbusmenuMenuitem * separator; > + > + separator = dbusmenu_menuitem_new(); > + dbusmenu_menuitem_property_set(separator, DBUSMENU_MENUITEM_PROP_TYPE, > DBUSMENU_CLIENT_TYPES_SEPARATOR); > + dbusmenu_menuitem_child_append(root, separator); > > - DbusmenuMenuitem * separator = dbusmenu_menuitem_new(); > + // 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) { > + // TODO Create "Add appointment" menu item > + } > + // TODO Create FFR? "Add timer" menu item > + > + separator = dbusmenu_menuitem_new(); > + dbusmenu_menuitem_property_set(separator, DBUSMENU_MENUITEM_PROP_TYPE, > DBUSMENU_CLIENT_TYPES_SEPARATOR); > + dbusmenu_menuitem_child_append(root, separator); > + > + update_timezone_menu_items(NULL); > + // TODO Create "Add location" menu item > + > + separator = dbusmenu_menuitem_new(); > dbusmenu_menuitem_property_set(separator, DBUSMENU_MENUITEM_PROP_TYPE, > DBUSMENU_CLIENT_TYPES_SEPARATOR); > dbusmenu_menuitem_child_append(root, separator); > > > === modified file 'src/dbus-shared.h' > --- src/dbus-shared.h 2010-08-31 12:57:01 +0000 > +++ src/dbus-shared.h 2011-01-26 11:50:11 +0000 > @@ -29,3 +29,12 @@ > > #define DBUSMENU_CALENDAR_MENUITEM_TYPE "x-canonical-calendar-item" > > +#define APPOINTMENT_MENUITEM_TYPE "appointment-item" > +#define APPOINTMENT_MENUITEM_PROP_LABEL "appointment-label" > +#define APPOINTMENT_MENUITEM_PROP_ICON "appointment-icon" > +#define APPOINTMENT_MENUITEM_PROP_RIGHT "appointment-time" > + > +#define TIMEZONE_MENUITEM_TYPE "timezone-item" > +#define TIMEZONE_MENUITEM_PROP_LABEL "timezone-label" > +#define TIMEZONE_MENUITEM_PROP_RADIO "timezone-radio" > +#define TIMEZONE_MENUITEM_PROP_RIGHT "timezone-time" > > === modified file 'src/indicator-datetime.c' > --- src/indicator-datetime.c 2011-01-14 06:19:05 +0000 > +++ src/indicator-datetime.c 2011-01-26 11:50:11 +0000 > @@ -41,6 +41,7 @@ > /* DBusMenu */ > #include <libdbusmenu-gtk/menu.h> > #include <libido/idocalendarmenuitem.h> > +#include <libdbusmenu-gtk/menuitem.h> > > #include "dbus-shared.h" > > @@ -102,6 +103,13 @@ > PROP_CUSTOM_TIME_FORMAT > }; > > +typedef struct _indicator_item_t indicator_item_t; > +struct _indicator_item_t { > + GtkWidget * icon; > + GtkWidget * label; > + GtkWidget * right; > +}; > + > #define PROP_TIME_FORMAT_S "time-format" > #define PROP_SHOW_SECONDS_S "show-seconds" > #define PROP_SHOW_DAY_S "show-day" > @@ -174,6 +182,8 @@ > > G_DEFINE_TYPE (IndicatorDatetime, indicator_datetime, INDICATOR_OBJECT_TYPE); > > +static GtkSizeGroup * indicator_right_group = NULL; > + > static void > indicator_datetime_class_init (IndicatorDatetimeClass *klass) > { > @@ -1055,6 +1065,128 @@ > return g_strdup_printf(T_("%s, %s"), date_string, time_string); > } > > +/* Whenever we have a property change on a DbusmenuMenuitem > + we need to be responsive to that. */ > +static void > +indicator_prop_change_cb (DbusmenuMenuitem * mi, gchar * prop, gchar * > value, indicator_item_t * mi_data) > +{ > + if (!g_strcmp0(prop, APPOINTMENT_MENUITEM_PROP_LABEL)) { > + /* Set the main label */ > + gtk_label_set_text(GTK_LABEL(mi_data->label), value); > + } else if (!g_strcmp0(prop, APPOINTMENT_MENUITEM_PROP_RIGHT)) { > + /* Set the right label */ > + gtk_label_set_text(GTK_LABEL(mi_data->right), value); > + } else if (!g_strcmp0(prop, APPOINTMENT_MENUITEM_PROP_ICON)) { > + /* We don't use the value here, which is probably less > efficient, > + but it's easier to use the easy function. And since th value > + is already cached, shouldn't be a big deal really. */ > + GdkPixbuf * pixbuf = dbusmenu_menuitem_property_get_image(mi, > APPOINTMENT_MENUITEM_PROP_ICON); > + if (pixbuf != NULL) { > + /* If we've got a pixbuf we need to make sure it's of a > reasonable > + size to fit in the menu. If not, rescale it. */ > + GdkPixbuf * resized_pixbuf; > + gint width, height; > + gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, > &height); > + if (gdk_pixbuf_get_width(pixbuf) > width || > + gdk_pixbuf_get_height(pixbuf) > height) > { > + g_debug("Resizing icon from %dx%d to %dx%d", > gdk_pixbuf_get_width(pixbuf), gdk_pixbuf_get_height(pixbuf), width, height); > + resized_pixbuf = gdk_pixbuf_scale_simple(pixbuf, > + width, > + height, > + > GDK_INTERP_BILINEAR); > + } else { > + g_debug("Happy with icon sized %dx%d", > gdk_pixbuf_get_width(pixbuf), gdk_pixbuf_get_height(pixbuf)); > + resized_pixbuf = pixbuf; > + } > + gtk_image_set_from_pixbuf(GTK_IMAGE(mi_data->icon), > resized_pixbuf); > + /* The other pixbuf should be free'd by the dbusmenu. */ > + if (resized_pixbuf != pixbuf) { > + g_object_unref(resized_pixbuf); > + } > + } > + } else { > + g_warning("Indicator Item property '%s' unknown", prop); > + } > + return; > +} > + > +/* We have a small little menuitem type that handles all > + of the fun stuff for indicators. Mostly this is the > + shifting over and putting the icon in with some right > + side text that'll be determined by the service. > + Copied verbatim from an old revision (including comments) of > indicator-messages > +*/ > +static gboolean > +new_appointment_item (DbusmenuMenuitem * newitem, DbusmenuMenuitem * parent, > DbusmenuClient * client) > +{ > + g_return_val_if_fail(DBUSMENU_IS_MENUITEM(newitem), FALSE); > + g_return_val_if_fail(DBUSMENU_IS_GTKCLIENT(client), FALSE); > + /* Note: not checking parent, it's reasonable for it to be NULL */ > + > + indicator_item_t * mi_data = g_new0(indicator_item_t, 1); > + > + GtkMenuItem * gmi = GTK_MENU_ITEM(gtk_menu_item_new()); > + > + GtkWidget * hbox = gtk_hbox_new(FALSE, 4); > + > + /* Icon, probably someone's face or avatar on an IM */ > + mi_data->icon = gtk_image_new(); > + GdkPixbuf * pixbuf = dbusmenu_menuitem_property_get_image(newitem, > APPOINTMENT_MENUITEM_PROP_ICON); > + > + if (pixbuf != NULL) { > + /* If we've got a pixbuf we need to make sure it's of a > reasonable > + size to fit in the menu. If not, rescale it. */ > + GdkPixbuf * resized_pixbuf; > + gint width, height; > + gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height); > + if (gdk_pixbuf_get_width(pixbuf) > width || > + gdk_pixbuf_get_height(pixbuf) > height) { > + g_debug("Resizing icon from %dx%d to %dx%d", > gdk_pixbuf_get_width(pixbuf), gdk_pixbuf_get_height(pixbuf), width, height); > + resized_pixbuf = gdk_pixbuf_scale_simple(pixbuf, > + width, > + height, > + > GDK_INTERP_BILINEAR); > + } else { > + g_debug("Happy with icon sized %dx%d", > gdk_pixbuf_get_width(pixbuf), gdk_pixbuf_get_height(pixbuf)); > + resized_pixbuf = pixbuf; > + } > + > + gtk_image_set_from_pixbuf(GTK_IMAGE(mi_data->icon), > resized_pixbuf); > + > + /* The other pixbuf should be free'd by the dbusmenu. */ > + if (resized_pixbuf != pixbuf) { > + g_object_unref(resized_pixbuf); > + } > + } > + gtk_misc_set_alignment(GTK_MISC(mi_data->icon), 0.0, 0.5); > + gtk_box_pack_start(GTK_BOX(hbox), mi_data->icon, FALSE, FALSE, 0); > + gtk_widget_show(mi_data->icon); > + > + /* Label, probably a username, chat room or mailbox name */ > + mi_data->label = gtk_label_new(dbusmenu_menuitem_property_get(newitem, > APPOINTMENT_MENUITEM_PROP_LABEL)); > + gtk_misc_set_alignment(GTK_MISC(mi_data->label), 0.0, 0.5); > + gtk_box_pack_start(GTK_BOX(hbox), mi_data->label, TRUE, TRUE, 0); > + gtk_widget_show(mi_data->label); > + > + /* Usually either the time or the count on the individual > + item. */ > + mi_data->right = gtk_label_new(dbusmenu_menuitem_property_get(newitem, > APPOINTMENT_MENUITEM_PROP_RIGHT)); > + gtk_size_group_add_widget(indicator_right_group, mi_data->right); > + gtk_misc_set_alignment(GTK_MISC(mi_data->right), 1.0, 0.5); > + gtk_box_pack_start(GTK_BOX(hbox), mi_data->right, FALSE, FALSE, 0); > + gtk_widget_show(mi_data->right); > + > + gtk_container_add(GTK_CONTAINER(gmi), hbox); > + gtk_widget_show(hbox); > + > + dbusmenu_gtkclient_newitem_base(DBUSMENU_GTKCLIENT(client), newitem, > gmi, parent); > + > + g_signal_connect(G_OBJECT(newitem), > DBUSMENU_MENUITEM_SIGNAL_PROPERTY_CHANGED, > G_CALLBACK(indicator_prop_change_cb), mi_data); > + g_signal_connect(G_OBJECT(newitem), "destroyed", G_CALLBACK(g_free), > mi_data); > + > + return TRUE; > +} > + > static gboolean > new_calendar_item (DbusmenuMenuitem * newitem, > DbusmenuMenuitem * parent, > @@ -1081,6 +1213,16 @@ > return TRUE; > } > > +static gboolean > +new_timezone_item(DbusmenuMenuitem * newitem, > + DbusmenuMenuitem * parent, > + DbusmenuClient * client) > +{ > + // Menu item with a radio button and a right aligned time > + > + return TRUE; > +} > + > /* Grabs the label. Creates it if it doesn't > exist already */ > static GtkLabel * > @@ -1120,6 +1262,8 @@ > g_object_set_data (G_OBJECT (client), "indicator", io); > > dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), > DBUSMENU_CALENDAR_MENUITEM_TYPE, new_calendar_item); > + dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), > APPOINTMENT_MENUITEM_TYPE, new_appointment_item); > + dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), > TIMEZONE_MENUITEM_TYPE, new_timezone_item); > > return GTK_MENU(self->priv->menu); > } > -- https://code.launchpad.net/~karl-qdh/ubuntu/natty/indicator-datetime/indicator-datetime.withappointments/+merge/47514 Your team ayatana-commits is subscribed to branch lp:indicator-datetime. _______________________________________________ Mailing list: https://launchpad.net/~ayatana-commits Post to : [email protected] Unsubscribe : https://launchpad.net/~ayatana-commits More help : https://help.launchpad.net/ListHelp
