GNOME Bugzilla – Bug 780629
Some date formats hardcoded as "month day"
Last modified: 2017-06-01 14:39:15 UTC
This piece of code: str = g_strdup_printf ("%s %d", gcal_get_month_name (...), day); causes the date to be always displayed as month + " " + day. This is correct in English but not correct in many other languages which often put a day before a month name. Additionally, languages would prefer adding a dot after a day number, maybe they want no space, etc. As a minimum I suggest making the format string translatable + adding an instruction for translators how they should use format arguments repositioning, for example they should use "%2$d %1$s" if they want to achieve "day month" order. However, I'm afraid it may be too tricky for translators. For a perfect solution I'd suggest using something sprintf-compatible, for example g_date_time_format(). This would require converting the date to GDateTime before. This bug appears in two places in gnome-calendar: - rebuild_popover_for_day() in gcal-month-view.c:224, - update_sidebar_headers() in gcal-year-view.c:501.
Created attachment 348902 [details] [review] Proposed solution This turned out to be easier than I initially thought it would be. No additional memory allocation. Please review, test, feel free to adopt to the customs of this project. Enjoy!
Are you sure you want to use “%-d” instead of “%d”? I’ve never seen it used in the source strings, only in translations. Other than that, this is an important bug fix and I’m looking forward to seeing it integrated!
g_date_time_format (…, "%B %-d") has literally the same effect as the original g_strdup_printf ("%s %d",…): month name + " " + day as a regular integer number. "%d" in g_date_time_format () generates a day with an optional trailing zero. If this was what the original authors wanted they would use g_strdup_printf ("%s %02d",…). So yes, this is what I want to use. Now I'm not sure if this is really what the original authors wanted to achieve. But it's a good opportunity for them to speak about it. :-)
(In reply to Piotr Drąg from comment #2) > Other than that, this is an important bug fix and I’m looking forward to > seeing it integrated! Should this be considered a translation issue? If so, I'll push this fix to stable branches too.
I’d consider it to be “strings not marked for translation by accident”, so it would be nice to have the fix in active stable branches.
Georges, questions to you before you push: 1. g_strdup_printf ("%s %d", …) formats the date like "April 9" while g_date_time_format (…, "%B %d") formats it like "April 09". Is this intentional? If you wanted "April 09" you should have used "%s %02d", if you wanted "April 9" you should have used "%B %-d". 2. You already have "%B %d" even if 3.24 branch, for example: https://git.gnome.org/browse/gnome-calendar/tree/src/gcal-quick-add-popover.c?h=gnome-3-24#n234 but it has "event date format" context. Why this context if there is no other context and also no "%B %d" format without any context in this application? Was it different in the past or do you plan it will be different in the future? My suggestions are: - Please rethink whether you want trailing zeros ("%B %d") or no trailing zeros ("%B %-d") or maybe trailing spaces ("%B %e"), - also please rethink whether you really need "event date format" context, - for the master branch: please rework the patch to use the day number padding which you want, also please consider removing "event date format" context where it exists unless you really want to let the translators translate "%B %d" in multiple ways, - for the gnome-3-24 branch: please rework this patch to use "event date format" so it will reuse the existing strings which are already correctly translated by the translators unless you really don't want to reuse the same translations, - or please explain what you want and ask me to rework the patch according to your needs.
Review of attachment 348902 [details] [review]: Answering your questions: - I want the trailing zeroes (i.e. use "%d") - I don't mind removing the context. I guess it makes sense to remove it. ::: src/views/gcal-month-view.c @@ +321,3 @@ + g_object_set_data (G_OBJECT (self->overflow_popover), "selected-day", + GINT_TO_POINTER (g_date_time_get_day_of_month (day))); One argument per line, please ::: src/views/gcal-year-view.c @@ +283,3 @@ + /* Translators: This is a date format in the sidebar of the year + * view when there is only one specified day selected. */ + : _("%B %-d")); Since there are translator comments, I'd rather have an if(...)/else here.
Created attachment 352680 [details] [review] Proposed solution (v2) Done!
Comment on attachment 352680 [details] [review] Proposed solution (v2) ++
Attachment 352680 [details] pushed as 2263fc9 - date formats: Use g_date_time_format()