ayatana-commits team mailing list archive

[Ted Gould <ted@xxxxxxxx>]

Review: Needs Fixing
On Tue, 2011-03-08 at 15:46 +0000, Karl Lattimer wrote:
> +	t2 = t1 + (time_t) (7 * 24 * 60 * 60); /* 7 days ahead of now, we actually need number_of_days_in_this_month */

Could you please make this comment a TODO.

> +	} else if (!g_strcmp0(prop, CALENDAR_MENUITEM_PROP_SET_DATE)) {
> +		// TODO This needs to be an array of 3 ints
> +		//ido_calendar_menu_item_set_date (IDO_CALENDAR_MENU_ITEM (mi_data), );

Confused here... use an array of 3 ints then? :)
 
> +static void
> +day_selected_double_click_cb (IdoCalendarMenuItem *ido,
> +				              guint           day,
> +                              gpointer        user_data) 
> +{
> +	gchar datestring[20];
> +	guint d,m,y;
> +	DbusmenuMenuitem * item = DBUSMENU_MENUITEM (user_data);
> +	ido_calendar_menu_item_get_date(ido, &y, &m, &d);
> +	g_sprintf(datestring, "%d-%d-%d", y, m, d);
> +	GVariant *variant = g_variant_new_string(datestring);
> +	guint timestamp = (guint)time(NULL);
> +	dbusmenu_menuitem_handle_event(DBUSMENU_MENUITEM(item), "day-selected-double-click", variant, timestamp);
> +	g_debug("Got day-selected-double-click signal: %s", datestring);
> +}
> +*/

Why are you encoding the data into a string instead of just passing an
array of ints?  Seems it would be better to let GVariant do the encoding
for us.

> +	dbusmenu_gtkclient_newitem_base(DBUSMENU_GTKCLIENT(client), newitem, GTK_MENU_ITEM(ido), parent);
> +	/*g_signal_connect_after(ido, "day-selected", G_CALLBACK(day_selected_cb), (gpointer)newitem);
> +	dbusmenu_gtkclient_newitem_base(DBUSMENU_GTKCLIENT(client), newitem, GTK_MENU_ITEM(ido), parent);
> +	g_signal_connect_after(ido, "day-selected-double-click", G_CALLBACK(day_selected_double_click_cb), (gpointer)newitem);*/

_newitem_base() should only need to be called once here.

  review needsfixing




-- 
https://code.launchpad.net/~karl-qdh/indicator-datetime/calendarmenuitemsignals/+merge/52570
Your team ayatana-commits is subscribed to branch lp:indicator-datetime.