Updates to TimeControl.durationString() #1342
Conversation
…the online-go codebase and it is just a wrapper for TimeControl.timeControlDescription()
Most, if not all, calls to durationString are followed by toLowerCase(). This causes difficulty in German translation because nouns are always capitalized regardless of location in the sentence. This commit allows translators to control the capitalization of the time words "week", "day", "hour", "minute" and "second". In addition, the internals of durationString() have been updated to allow for re-ordering of the units, as might be the case in right-to-left languages. This puts a higher workload on the translators, but gives more flexibility.
Some languages, such as German, capitalize words even if they are in the middle of a sentence. This commit gives translators the ability to choose how the time words ("days", "hours", "minutes", etc.) are capitalized.
…arately. - RTL languages still have the option to change order through coarse_fine_time_template. - durationString is not a little DRYer by making generic coarse/fine code instead of week/day, day/hour, hour/minute etc.
…ersa. ngettext was a little too smart, causing "1 day" to be translated to "1 days" in durationString() because "%s days" was in the catalog, but not "%s day". Since there is no English project at translate.online-go.com, there was no way to fix this besides changing "%s day" to "1 day", which causes potential issues with interpolate().
|
Initially, I thought this PR might create undue load on translators. However, with the last two commits, their work should be simplified. |
| @@ -155,10 +155,10 @@ export function ngettext(singular: string, plural: string, count: number) { | |||
|
|
|||
| /* If we don't have a ngettext translation entry at all, but | |||
| * we do have some stand alone translations, use those */ | |||
| if ((count !== 1 || !(singular in catalog)) && plural in catalog) { | |||
| if (count !== 1 && plural in catalog) { | |||
There was a problem hiding this comment.
I'm not entirely sure if the way we were doing it was particularly useful, so I am fine with this change as it's "more correct" I reckon - but just curious, was this a noted problem somewhere or why the change?
There was a problem hiding this comment.
Granted, there was no issue before the updates in this PR. However, it manifested itself like so:
The issue was that ngettext was detecting a _() translation of "%s days" but not "%s day". I think it was borrowing the translation from util.mktime(), which has literals _("1 day") and _("%s days"). To be honest, I wasn't sure about the best way to fix this.
Things I considered
- Check the language in ngettext `if (currentLanguage() != 'en' && ...)
- Add a translation in English
- I see catalog is populated in English/debug, but I couldn't figure out how this works since there is no English project on translate.online-go.com
- Call
interpolate(ngettext("1 day", "%s days", ...- This would work, but would produce a warning
- Remove some of the "smarts" of
ngettext
I went with the last option because it seemed the simplest and would prevent spooky-action-at-a-distance errors like the one I ran into.
|
This looks great, thanks for the updates around this :) One quick fix so the string replacement works and a quick sanity check on the translation fallback code and I think we're good to go. |
|
Thanks for the review! That commit should address your first note. Hopefully my comment on the second note explains what I was trying to do, but I'm open to discussion on a different fix. |
|
Awesome, yeah that all makes sense, and thanks for the details! |
After translation updates, this will fix one of the German language localization issues presented in the OGS Forum thread message not localized (unremovable branch, time control)
Proposed Changes
durationStringdurationStringnow returns lowercase (if not translated)