The Date and Time Indicator

Merge lp:~gunnarhj/indicator-datetime/days-months into lp:indicator-datetime/13.10

Proposed by Gunnar Hjalmarsson on 2013-04-16
Status: Work in progress
Proposed branch: lp:~gunnarhj/indicator-datetime/days-months
Merge into: lp:indicator-datetime/13.10
Diff against target: 133 lines (+80/-3) 5 files modified
To merge this branch: bzr merge lp:~gunnarhj/indicator-datetime/days-months
Reviewer Review Type Date Requested Status
Martin Pitt (community) 2013-04-16 Approve on 2013-06-20
Mathieu Trudel-Lapierre 2013-04-16 Approve on 2013-06-12
Ubuntu branches 2013-04-16 Pending
Charles Kerr 2013-04-16 Pending
Review via email: mp+159214@code.launchpad.net

Commit Message

src/utils.c:
Added the format_date_time() function, which converts names of
days and months using the current language (LP: #1024663,
LP: #1072019, LP: #1160441).

Description of the Change

Unfortunately Matthias has added WONTFIX to https://bugzilla.gnome.org/show_bug.cgi?id=687945, so I take it it's time to reconsider the possibility to do this at the indicator-datetime level, after all.

In my PPA at https://launchpad.net/~gunnarhj/+archive/misc there are both Quantal and Raring builds of indicator-datetime including the proposed patch, for the case you want to test.

Charles, last Tuesday you said a couple of things on IRC that I didn't quite understand:

"if we do this at the indicator level, we could use your code to make some namespace-appropriate version of the g_date_time_format() that you've got in that bgo patch
iirc the datetime patch we had before relied on some assumptions about the format strings that we pass into strftime, and those might change:
at some point we may want to expose a format string config option in dconf-editor, though not visible in g-c-c"

Please note that the name of the function I propose is format_date_time(), i.e. no "g_" prefix.

To post a comment you must log in.
Gunnar Hjalmarsson (gunnarhj) wrote :

Thanks, Mathieu. Those user expectations are natural and legitimate, and needless to say I think that an SRU later on would be a good idea. ;-)

Gunnar Hjalmarsson (gunnarhj) wrote :

Before merging this, I suppose that somebody should merge revision 212-214 from lp:indicator-datetime/13.04.

I'm going to wait for now, just to have the time to test it and see on my own mixed locale setup :)

Gunnar Hjalmarsson (gunnarhj) wrote :

Of course. I am successfully using the build in my PPA: https://launchpad.net/~gunnarhj/+archive/misc

212. By Gunnar Hjalmarsson on 2013-04-22

src/utils.c:
Added the format_date_time() function, which converts names of
days and months using the current language (LP: #1024663,
LP: #1072019, LP: #1160441).

Iain Lane (laney) wrote :

Ping cyphermox - this has been pending for some time :-)

Patch looks correct, and works on my system. This will bring the datetime indicator to get translated date strings in the same language as the rest of the UI; which is the "best" we can do considering the current situation with the LC_TIME POSIX category. It will still use LC_TIME for the ordering of the bits of information and time format.

While this is a departure from the standard, and so suboptimal (doesn't fix 'date', for instance, which will still use LC_TIME to display everything), it brings us a bit closer to what users may reasonably expect -- an interface all in the same language.

review: Approve
Martin Pitt (pitti) wrote :

I still consider it a bit of a hack; it would be much better to get this consistent in glib itself, but as we won't get that, I'm fine with adding this locally to indicator-datetime as it is so prominent on the desktop. Thanks!

review: Approve
Charles Kerr (charlesk) wrote :

Overall I agree with Martin on this -- it's kind of hacky, but a net plus for indicator-datetime.

Gunnar, as we discussed in #ubuntu-devel this patch needs to be updated to fit the GMenuified version of datetime that's about to land.

 1. Some of the time formatting is about the same as before: the header (ie, the date shown in the panel) and the first menuitem (today's date) are implemented about the same as before, so updating the utils.c patch for these two pieces should be pretty easy.

 2. However the appointment and location menuitems are very different. To avoid sending excess menuitem updates over the bus, we pass date format strings over the bus for these menuitems and let the client code do its own periodic calls to strftime(). For these, you'll want to modify the code in datetime/src/service.c such that the "x-canonical-time-format" properties set in create_appointments_section() and create_locations_section() contain your conversions.

Comments on format_date_time():

 1. when formatted_token is NULL, it's probably better to use the unformatted token than to bomb out. That way the user will get a partially formatted string instead of nothing, and will get a hint about which part of the formatting went wrong.

 2. splitting on " " seems a little brittle, as it'll fail on unconventional format strings. It might be better to create a GString to hold the output, then walk the input string, watching for '%'. During the walk the GString would be fed the normal characters from the input string, and be fed formatted strings whenever any of dm_names were reached in the input string. (This is just one idea, it doesn't have to be done exactly this way; I'm just giving an example on how to pass even on something like "%A,%B")

Unmerged revisions

212. By Gunnar Hjalmarsson on 2013-04-22

src/utils.c:
Added the format_date_time() function, which converts names of
days and months using the current language (LP: #1024663,
LP: #1072019, LP: #1160441).

Preview Diff

1=== modified file 'debian/changelog'
2--- debian/changelog 2013-03-07 02:01:22 +0000
3+++ debian/changelog 2013-04-22 23:02:26 +0000
4@@ -1,3 +1,12 @@
5+indicator-datetime (12.10.3daily13.03.07-0ubuntu2) UNRELEASED; urgency=low
6+
7+ * src/utils.c:
8+ - Added the format_date_time() function, which converts names of
9+ days and months using the current language (LP: #1024663,
10+ LP: #1072019, LP: #1160441).
11+
12+ -- Gunnar Hjalmarsson <gunnarhj@ubuntu.com> Tue, 23 Apr 2013 00:57:00 +0200
13+
14 indicator-datetime (12.10.3daily13.03.07-0ubuntu1) raring; urgency=low
15
16 [ Michael Terry ]
17
18=== modified file 'src/datetime-service.c'
19--- src/datetime-service.c 2013-02-04 15:13:42 +0000
20+++ src/datetime-service.c 2013-04-22 23:02:26 +0000
21@@ -349,7 +349,7 @@
22 }
23
24 /* eranslators: strftime(3) style date format on top of the menu when you click on the clock */
25- utf8 = g_date_time_format (datetime, _("%A, %e %B %Y"));
26+ utf8 = format_date_time (datetime, _("%A, %e %B %Y"));
27
28 dbusmenu_menuitem_property_set(date, DBUSMENU_MENUITEM_PROP_LABEL, utf8);
29
30
31=== modified file 'src/indicator-datetime.c'
32--- src/indicator-datetime.c 2012-10-04 15:53:00 +0000
33+++ src/indicator-datetime.c 2013-04-22 23:02:26 +0000
34@@ -741,11 +741,11 @@
35 gchar * timestr;
36 if (format == NULL) {
37 gchar * format_for_time = generate_format_string_at_time(datetime_now);
38- timestr = g_date_time_format(datetime_now, format_for_time);
39+ timestr = format_date_time(datetime_now, format_for_time);
40 g_free(format_for_time);
41 }
42 else {
43- timestr = g_date_time_format(datetime_now, format);
44+ timestr = format_date_time(datetime_now, format);
45 if (timestr == NULL) {
46 g_warning ("The custom date format is not valid, check the\n"
47 "g_date_time_format() documentation for the supported\n"
48
49=== modified file 'src/utils.c'
50--- src/utils.c 2012-05-25 20:42:25 +0000
51+++ src/utils.c 2013-04-22 23:02:26 +0000
52@@ -301,3 +301,70 @@
53 return generate_format_string_full(show_day, show_date);
54 }
55
56+/* Translate names of days and months using the current language by
57+ splitting a format specification and applying a suitable locale
58+ when converting respective token. */
59+gchar *
60+format_date_time (GDateTime *datetime_now, const gchar *format_spec)
61+{
62+ gchar *formatted_string = NULL;
63+ gchar *tmp = NULL;
64+ gint i, j;
65+
66+ /* Probably unnecessary, but just in case... */
67+ if (format_spec == NULL || strlen (format_spec) == 0)
68+ return NULL;
69+
70+ gchar **tokens = g_strsplit (format_spec, " ", 0);
71+
72+ /* Conversion specifications for names of days and months */
73+ const gchar *dm_names[] = { "%a", "%A", "%b", "%B", "%h", NULL };
74+
75+ /* Get some locale info, which in turn was taken from the
76+ locale environment via the setlocale(LC_ALL, "") call in
77+ main() in src/datetime-service.c. */
78+ const gchar *time_locale = setlocale (LC_TIME, NULL);
79+ const gchar *message_locale = setlocale (LC_MESSAGES, NULL);
80+
81+ for (i = 0; tokens[i] != NULL; i++)
82+ {
83+ gboolean is_day_month = FALSE;
84+
85+ for (j = 0; dm_names[j] != NULL; j++) {
86+ if (g_strrstr (tokens[i], dm_names[j]) != NULL) {
87+ is_day_month = TRUE;
88+ break;
89+ }
90+ }
91+
92+ if (is_day_month)
93+ setlocale (LC_TIME, message_locale);
94+ else
95+ setlocale (LC_TIME, time_locale);
96+
97+ gchar *formatted_token = g_date_time_format (datetime_now, tokens[i]);
98+ if (formatted_token == NULL) {
99+ g_free (formatted_token);
100+ goto out;
101+ }
102+
103+ if (i == 0)
104+ tmp = g_strdup (formatted_token);
105+ else
106+ tmp = g_strjoin (" ", tmp, formatted_token, NULL);
107+
108+ g_free (formatted_token);
109+ }
110+
111+ formatted_string = g_strdup (tmp);
112+
113+out:
114+ g_strfreev (tokens);
115+ g_free (tmp);
116+
117+ /* Reset the locale */
118+ setlocale (LC_TIME, time_locale);
119+
120+ return formatted_string;
121+}
122+
123
124=== modified file 'src/utils.h'
125--- src/utils.h 2011-06-28 14:32:04 +0000
126+++ src/utils.h 2013-04-22 23:02:26 +0000
127@@ -33,6 +33,7 @@
128 gchar * read_timezone ();
129 gchar * generate_format_string_full (gboolean show_day, gboolean show_date);
130 gchar * generate_format_string_at_time (GDateTime * time);
131+gchar * format_date_time (GDateTime *datetime_now, const gchar *format_spec);
132
133 G_END_DECLS
134

Subscribers

People subscribed via source and target branches