GNOME Bugzilla – Bug 790416
g_date_time_format returns empty string on %r with German locale
Last modified: 2017-11-28 14:27:27 UTC
When using g_date_time_format with German locale settings for time the %r format specifier does not work and returns an empty string. While I attributed that to "AM" and "PM" not being part of the German locale or time format, a call to "%I:%M %p" works as expected. Also, calling "date +%r" in the terminal works out fine with the same settings. This behavior was discovered as part of the xfce4-panel: https://bugzilla.xfce.org/show_bug.cgi?id=11527
Naturally, for de_DE the result of nl_langinfo(T_FMT_AMPM) is "". So the output of g_date_time_format() is "" too. I guess it could instead return NULL since that's the documented return value in case of an error.
Created attachment 363812 [details] [review] gdatetime: Fix handling of unsupported nl_langinfo() items If nl_langinfo() doesn’t support a particular item, it returns the empty string. We should check for that and return NULL from g_date_time_format() accordingly, otherwise the user could unwittingly end up with a formatted date/time which is missing some or all of its components. This arose with %r in de_DE, which is unsupported by nl_langinfo() because Germans almost never write time in 12-hour format. Add a unit test. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Created attachment 363813 [details] [review] gdatetime: Drop a duplicate #define It’s exactly the same as the one on the next line. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Review of attachment 363812 [details] [review]: The implementation looks good. ::: glib/tests/gdatetime.c @@ +1836,3 @@ + } + else + g_test_message ("locale de_DE not available, skipping error handling tests"); Shouldn't you use g_test_skip()?
Review of attachment 363813 [details] [review]: ACK-by: me.
(In reply to Emmanuele Bassi (:ebassi) from comment #4) > ::: glib/tests/gdatetime.c > @@ +1836,3 @@ > + } > + else > + g_test_message ("locale de_DE not available, skipping error handling > tests"); > > Shouldn't you use g_test_skip()? Oops, yes. When I wrote this patch, the rest of the file used g_test_message(). Then I did 0dc68e5d469654d6e4b35d73f48c9ec550dcdfb4 and changed them all to g_test_skip(), but forgot to update this patch. Thanks for catching it. I’ll push with it changed to g_test_skip().
Attachment 363812 [details] pushed as bccc105 - gdatetime: Fix handling of unsupported nl_langinfo() items Attachment 363813 [details] pushed as 643c2d5 - gdatetime: Drop a duplicate #define