Merge lp:~charlesk/indicator-datetime/fix-833337 into lp:indicator-datetime/0.4

Proposed by Ted Gould on 2012-03-03
Status: Merged
Approved by: Ted Gould on 2012-03-03
Approved revision: 169
Merged at revision: 156
Proposed branch: lp:~charlesk/indicator-datetime/fix-833337
Merge into: lp:indicator-datetime/0.4
Prerequisite: lp:~charlesk/indicator-datetime/fix-leaks
Diff against target: 406 lines (+141/-188)
1 file modified
src/datetime-service.c (+141/-188)
To merge this branch: bzr merge lp:~charlesk/indicator-datetime/fix-833337
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve on 2012-03-03
Review via email: mp+95719@code.launchpad.net

This proposal supersedes a proposal from 2012-03-02.

Description of the change

This addresses Bug #833337 (Duplicate locations) and Bug #833325 (locations not in time order).

We have three sources that we draw location candidates from:
  - our geographic location, drawn from geoclue
  - our current location, drawn from /etc/timezone
  - the user-specified locations in g-c-c -> Time & Date -> Clock -> Time in other locations + Choose Locations

This patch builds a list from these three sources, and omits duplicates by testing for entries that have the same 'name' property *and* the same timezone offset from UTC. So if there were entries for Amsterdam NY and Amsterdam Netherlands, both would be used. But if there were two entries for Amsterdam NY, one would be omitted.

The list is then sorted in timezone order for Bug #833325, and DbusmenuMeuitems are created for each item in the list.

This code is branched from lp:~charlesk/indicator-datetime/fix-leaks, which does what it says on the tin.

To post a comment you must log in.
Ted Gould (ted) wrote :

Resubmitted to make the fix-leaks branch a prereq branch

Ted Gould (ted) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/datetime-service.c'
2--- src/datetime-service.c 2012-03-03 04:10:23 +0000
3+++ src/datetime-service.c 2012-03-03 04:10:24 +0000
4@@ -68,6 +68,8 @@
5 static void geo_address_change (GeoclueMasterClient * client, gchar * a, gchar * b, gchar * c, gchar * d, gpointer user_data);
6 static gboolean get_greeter_mode (void);
7
8+static void quick_set_tz (DbusmenuMenuitem * menuitem, guint timestamp, gpointer user_data);
9+
10 static IndicatorService * service = NULL;
11 static GMainLoop * mainloop = NULL;
12 static DbusmenuServer * server = NULL;
13@@ -80,12 +82,9 @@
14 static DbusmenuMenuitem * settings = NULL;
15 static DbusmenuMenuitem * events_separator = NULL;
16 static DbusmenuMenuitem * locations_separator = NULL;
17-static DbusmenuMenuitem * geo_location = NULL;
18-static DbusmenuMenuitem * current_location = NULL;
19-//static DbusmenuMenuitem * ecal_location = NULL;
20 static DbusmenuMenuitem * add_appointment = NULL;
21 static GList * appointments = NULL;
22-static GList * dconflocations = NULL;
23+static GList * location_menu_items = NULL;
24 static GList * comp_instances = NULL;
25 static gboolean updating_appointments = FALSE;
26 static time_t start_time_appointments = (time_t) 0;
27@@ -108,115 +107,140 @@
28 ESource *source;
29 };
30
31-static void
32-set_timezone_label (DbusmenuMenuitem * mi, const gchar * location)
33-{
34- gchar * zone, * name;
35- split_settings_location (location, &zone, &name);
36-
37- dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_NAME, name);
38- dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_ZONE, zone);
39-
40- g_free (zone);
41- g_free (name);
42-}
43-
44-static void
45-set_current_timezone_label (DbusmenuMenuitem * mi, const gchar * location)
46-{
47- gchar * name = get_current_zone_name (location);
48-
49- dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_NAME, name);
50- dbusmenu_menuitem_property_set (mi, TIMEZONE_MENUITEM_PROP_ZONE, location);
51-
52- g_free (name);
53-}
54-
55-/* Check to see if our timezones are the same */
56-static void
57-check_timezone_sync (void) {
58- gchar * label;
59- gboolean in_sync = FALSE;
60-
61- if (geo_timezone == NULL) {
62- in_sync = TRUE;
63- }
64-
65- if (current_timezone == NULL) {
66- in_sync = TRUE;
67- }
68-
69- if (!in_sync && g_strcmp0(geo_timezone, current_timezone) == 0) {
70- in_sync = TRUE;
71- }
72-
73- if (in_sync) {
74- g_debug("Timezones in sync");
75+/**
76+ * A temp struct used by update_location_menu_items() for pruning duplicates.
77+ */
78+struct TimeLocation
79+{
80+ gint32 offset;
81+ gchar * zone;
82+ gchar * name;
83+};
84+static void
85+time_location_free (struct TimeLocation * loc)
86+{
87+ g_free (loc->name);
88+ g_free (loc->zone);
89+ g_free (loc);
90+}
91+static struct TimeLocation*
92+time_location_new (const char * zone, const char * name)
93+{
94+ struct TimeLocation * loc = g_new (struct TimeLocation, 1);
95+ GTimeZone * tz = g_time_zone_new (zone);
96+ loc->offset = g_time_zone_get_offset (tz, 0);
97+ loc->zone = g_strdup (zone);
98+ loc->name = g_strdup (name);
99+ g_time_zone_unref (tz);
100+ return loc;
101+}
102+static int
103+time_location_compare (const struct TimeLocation * a, const struct TimeLocation * b)
104+{
105+ int ret;
106+ if (a->offset != b->offset)
107+ ret = a->offset - b->offset;
108+ else
109+ ret = g_strcmp0 (a->name, b->name);
110+ g_debug ("%s comparing '%s' (%d) to '%s' (%d), returning %d", G_STRLOC, a->name, (int)a->offset, b->name, (int)b->offset, ret);
111+ return ret;
112+}
113+static GSList*
114+locations_add (GSList * locations, const char * zone, const char * name)
115+{
116+ struct TimeLocation * loc = time_location_new (zone, name);
117+
118+ if (g_slist_find_custom (locations, loc, (GCompareFunc)time_location_compare) == NULL) {
119+ g_debug ("%s Adding zone '%s', name '%s'", G_STRLOC, zone, name);
120+ locations = g_slist_prepend (locations, loc);
121 } else {
122- g_debug("Timezones are different");
123- }
124-
125- gboolean show = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S);
126-
127- if (geo_location != NULL && current_location != NULL) {
128- g_debug("Got timezone %s", current_timezone);
129- g_debug("Got timezone %s", geo_timezone);
130- // Show neither current location nor geo location if both are the same
131- // however, we want to set their time and label accordingly
132- if (in_sync) {
133- if (current_timezone == NULL && geo_timezone == NULL) {
134- dbusmenu_menuitem_property_set_bool(locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
135- dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
136- dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
137- update_location_menu_items();
138- return;
139- }
140-
141- dbusmenu_menuitem_property_set_bool (locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
142- dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
143- dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE);
144- dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
145- dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE);
146-
147- if (current_timezone != NULL) {
148- label = current_timezone;
149- } else {
150- label = geo_timezone;
151- }
152-
153- if (label != NULL) {
154- // TODO work out the current location name in a nice way
155- set_current_timezone_label (current_location, label);
156- // TODO work out the current time at that location
157- dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, show);
158- dbusmenu_menuitem_property_set_bool(current_location, TIMEZONE_MENUITEM_PROP_RADIO, TRUE);
159- } else {
160- g_debug("Label for current location is null, this shouldn't happen");
161- }
162- if (geo_timezone != NULL) {
163- // TODO work out the geo location name in a nice way
164- set_current_timezone_label (geo_location, geo_timezone);
165- // TODO work out the current time at that location
166- dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_VISIBLE, show);
167- }
168- } else {
169- // TODO work out the geo location name in a nice way
170- set_current_timezone_label (geo_location, geo_timezone);
171- // TODO work out the current time at that location
172- dbusmenu_menuitem_property_set_bool(geo_location, DBUSMENU_MENUITEM_PROP_VISIBLE, show);
173-
174- // TODO work out the current location name in a nice way
175- set_current_timezone_label (current_location, current_timezone);
176- // TODO work out the current time at that location
177- dbusmenu_menuitem_property_set_bool(current_location, TIMEZONE_MENUITEM_PROP_RADIO, TRUE);
178- dbusmenu_menuitem_property_set_bool(current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, show);
179- dbusmenu_menuitem_property_set_bool(locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, show);
180+ g_debug("%s Skipping duplicate zone '%s' name '%s'", G_STRLOC, zone, name);
181+ time_location_free (loc);
182+ }
183+ return locations;
184+}
185+
186+/* Update the timezone entries */
187+static void
188+update_location_menu_items (void)
189+{
190+ if (locations_separator == NULL)
191+ return;
192+
193+ GSList * locations = NULL;
194+
195+ /* remove the previous locations */
196+ while (location_menu_items != NULL) {
197+ DbusmenuMenuitem * item = DBUSMENU_MENUITEM(location_menu_items->data);
198+ location_menu_items = g_list_remove(location_menu_items, item);
199+ dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item));
200+ g_object_unref(G_OBJECT(item));
201+ }
202+
203+ /***
204+ **** Build a list of locations to add: use geo_timezone,
205+ **** current_timezone, and SETTINGS_LOCATIONS_S, but omit duplicates.
206+ ***/
207+
208+ /* maybe add geo_timezone */
209+ if (geo_timezone != NULL) {
210+ gchar * name = get_current_zone_name (geo_timezone);
211+ locations = locations_add (locations, geo_timezone, name);
212+ g_free (name);
213+ }
214+
215+ /* maybe add current_timezone */
216+ if (current_timezone != NULL) {
217+ gchar * name = get_current_zone_name (current_timezone);
218+ locations = locations_add (locations, current_timezone, name);
219+ g_free (name);
220+ }
221+
222+ /* maybe add the user-specified custom locations */
223+ gchar ** user_locations = g_settings_get_strv(conf, SETTINGS_LOCATIONS_S);
224+ if (user_locations != NULL) {
225+ gint i;
226+ const guint location_count = g_strv_length(user_locations);
227+ g_debug ("%s Found %u user-specified locations", G_STRLOC, location_count);
228+ for (i=0; i<location_count; i++) {
229+ gchar * zone;
230+ gchar * name;
231+ split_settings_location (user_locations[i], &zone, &name);
232+ locations = locations_add (locations, zone, name);
233+ g_free (name);
234+ g_free (zone);
235 }
236- }
237- g_debug("Finished checking timezone sync");
238- update_location_menu_items();
239-
240- return;
241+ g_strfreev (user_locations);
242+ user_locations = NULL;
243+ }
244+
245+ /* sort the list by timezone offset */
246+ locations = g_slist_sort (locations, (GCompareFunc)time_location_compare);
247+
248+ /* finally create menuitems for each location */
249+ gint offset = dbusmenu_menuitem_get_position (locations_separator, root)+1;
250+ const gboolean show_locations = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S);
251+ GSList * l;
252+ for (l=locations; l!=NULL; l=l->next) {
253+ struct TimeLocation * loc = l->data;
254+ g_debug("%s Adding location: zone '%s', name '%s'", G_STRLOC, loc->zone, loc->name);
255+ DbusmenuMenuitem * item = dbusmenu_menuitem_new();
256+ dbusmenu_menuitem_property_set (item, DBUSMENU_MENUITEM_PROP_TYPE, TIMEZONE_MENUITEM_TYPE);
257+ dbusmenu_menuitem_property_set (item, TIMEZONE_MENUITEM_PROP_NAME, loc->name);
258+ dbusmenu_menuitem_property_set (item, TIMEZONE_MENUITEM_PROP_ZONE, loc->zone);
259+ dbusmenu_menuitem_property_set_bool (item, TIMEZONE_MENUITEM_PROP_RADIO, FALSE);
260+ dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE);
261+ dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_VISIBLE, show_locations);
262+ dbusmenu_menuitem_child_add_position (root, item, offset++);
263+ g_signal_connect(G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, G_CALLBACK(quick_set_tz), NULL);
264+ location_menu_items = g_list_append (location_menu_items, item);
265+ time_location_free (loc);
266+ }
267+ g_slist_free (locations);
268+ locations = NULL;
269+
270+ /* if there's at least one item being shown, show the separator too */
271+ dbusmenu_menuitem_property_set_bool (locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, show_locations && (location_menu_items!=NULL));
272 }
273
274 /* Update the current timezone */
275@@ -235,7 +259,7 @@
276
277 g_debug("System timezone is: %s", current_timezone);
278
279- check_timezone_sync();
280+ update_location_menu_items();
281
282 return;
283 }
284@@ -566,62 +590,6 @@
285 return FALSE;
286 }
287
288-
289-static void
290-update_location_menu_items(void) {
291- g_debug("Updating timezone menu items");
292-
293- if (locations_separator == NULL || current_location == NULL) {
294- return;
295- }
296-
297- gchar ** locations = g_settings_get_strv(conf, SETTINGS_LOCATIONS_S);
298-
299- if (locations == NULL) {
300- g_debug("No locations configured (NULL)");
301- return;
302- }
303- const guint len = g_strv_length(locations);
304- g_debug("Found %u locations from %s", len, SETTINGS_LOCATIONS_S);
305-
306- /* Remove all of the previous locations */
307- if (dconflocations != NULL) {
308- while (dconflocations != NULL) {
309- DbusmenuMenuitem * item = DBUSMENU_MENUITEM(dconflocations->data);
310- // Remove all the existing menu items which are in dconflocations.
311- dconflocations = g_list_remove(dconflocations, item);
312- dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item));
313- g_object_unref(G_OBJECT(item));
314- }
315- }
316-
317- const gboolean show = g_settings_get_boolean (conf, SETTINGS_SHOW_LOCATIONS_S);
318- dbusmenu_menuitem_property_set_bool (locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, show);
319- dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, show);
320- dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE);
321-
322- gint i;
323- gint offset = dbusmenu_menuitem_get_position (current_location, root)+1;
324- for (i = 0; i < len; i++) {
325- // Iterate over configured places and add any which aren't already listed
326- if ((current_timezone == NULL || !g_str_has_prefix(locations[i], current_timezone)) &&
327- (geo_timezone == NULL || !g_str_has_prefix(locations[i], geo_timezone))) {
328- DbusmenuMenuitem *item;
329- g_debug("Adding timezone in update_timezones %s", locations[i]);
330- item = dbusmenu_menuitem_new();
331- dbusmenu_menuitem_property_set (item, DBUSMENU_MENUITEM_PROP_TYPE, TIMEZONE_MENUITEM_TYPE);
332- set_timezone_label (item, locations[i]);
333- dbusmenu_menuitem_property_set_bool (item, TIMEZONE_MENUITEM_PROP_RADIO, FALSE);
334- dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE);
335- dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_VISIBLE, show);
336- dbusmenu_menuitem_child_add_position (root, item, offset++);
337- g_signal_connect(G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, G_CALLBACK(quick_set_tz), NULL);
338- dconflocations = g_list_append(dconflocations, item);
339- }
340- }
341- g_strfreev (locations);
342-}
343-
344 // Authentication function
345 static gchar *
346 auth_func (ECal *ecal,
347@@ -1098,7 +1066,7 @@
348 show_locations_changed (void)
349 {
350 /* Re-calculate */
351- check_timezone_sync();
352+ update_location_menu_items();
353 }
354
355 static void
356@@ -1141,22 +1109,7 @@
357 dbusmenu_menuitem_property_set_bool (locations_separator, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
358 dbusmenu_menuitem_child_append(root, locations_separator);
359
360- geo_location = dbusmenu_menuitem_new();
361- dbusmenu_menuitem_property_set (geo_location, DBUSMENU_MENUITEM_PROP_TYPE, TIMEZONE_MENUITEM_TYPE);
362- set_current_timezone_label (geo_location, "");
363- dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_ENABLED, FALSE);
364- dbusmenu_menuitem_property_set_bool (geo_location, DBUSMENU_MENUITEM_PROP_VISIBLE, TRUE);
365- g_signal_connect(G_OBJECT(geo_location), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, G_CALLBACK(quick_set_tz), NULL);
366- dbusmenu_menuitem_child_append(root, geo_location);
367-
368- current_location = dbusmenu_menuitem_new();
369- dbusmenu_menuitem_property_set (current_location, DBUSMENU_MENUITEM_PROP_TYPE, TIMEZONE_MENUITEM_TYPE);
370- set_current_timezone_label (current_location, "");
371- dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_ENABLED, FALSE);
372- dbusmenu_menuitem_property_set_bool (current_location, DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
373- dbusmenu_menuitem_child_append(root, current_location);
374-
375- check_timezone_sync();
376+ update_location_menu_items();
377
378 g_signal_connect (conf, "changed::" SETTINGS_SHOW_LOCATIONS_S, G_CALLBACK (show_locations_changed), NULL);
379 g_signal_connect (conf, "changed::" SETTINGS_LOCATIONS_S, G_CALLBACK (show_locations_changed), NULL);
380@@ -1297,7 +1250,7 @@
381 geo_timezone = g_strdup((gchar *)tz_hash);
382 }
383
384- check_timezone_sync();
385+ update_location_menu_items();
386
387 return;
388 }
389@@ -1394,7 +1347,7 @@
390 geo_timezone = NULL;
391 }
392
393- check_timezone_sync();
394+ update_location_menu_items();
395
396 return;
397 }
398@@ -1416,7 +1369,7 @@
399 geo_timezone = NULL;
400 }
401
402- check_timezone_sync();
403+ update_location_menu_items();
404
405 return;
406 }

Subscribers

People subscribed via source and target branches