Merge lp://qastaging/~gunnarhj/indicator-datetime/days-months into lp://qastaging/~indicator-applet-developers/indicator-datetime/trunk.13.10

Proposed by Gunnar Hjalmarsson
Status: Work in progress
Proposed branch: lp://qastaging/~gunnarhj/indicator-datetime/days-months
Merge into: lp://qastaging/~indicator-applet-developers/indicator-datetime/trunk.13.10
Diff against target: 133 lines (+80/-3)
5 files modified
debian/changelog (+9/-0)
src/datetime-service.c (+1/-1)
src/indicator-datetime.c (+2/-2)
src/utils.c (+67/-0)
src/utils.h (+1/-0)
To merge this branch: bzr merge lp://qastaging/~gunnarhj/indicator-datetime/days-months
Reviewer Review Type Date Requested Status
Martin Pitt Approve
Mathieu Trudel-Lapierre Approve
Ubuntu branches Pending
Charles Kerr Pending
Review via email: mp+159214@code.qastaging.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.
Revision history for this message
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. ;-)

Revision history for this message
Gunnar Hjalmarsson (gunnarhj) wrote :

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

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

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

Revision history for this message
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

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).

Revision history for this message
Iain Lane (laney) wrote :

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

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

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
Revision history for this message
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
Revision history for this message
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

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

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches