Fix plural handling for translations #1424
Conversation
.arg() doesn't handle plurals. This change is incomplete, it breaks widgets such as the FloatSlider (label is not updated QString::arg warnings are issued)
Only source strings which require plural handling need to be translated as long as the source language is American English.
|
I guess that obsoletes my patch in #1423 :) |
|
Thanks for picking this up @Simran-B and @prokoudine, I misunderstood how Qt's plurals worked. You may have noticed this already, but this might get complicated with those |
|
Yeah, it's certainly not trivial. One could set the string in SetFormat() without calling tr() and do that dynamically sometime later. That might make the UI work, but I'm fairly sure that lupdate won't pick up the source string anymore. Pseudo code: setFormat("%n foo");
...
tr(getFormat(), nullptr, Value().toInt()); // <-- not a string literalMaybe an unused/unreachable piece of code could be added with the string literal so that it gets picked up by lupdate without interfering with get/setFormat? The easiest solution would be to not add plural handling for sliders and such however. |
|
Yeah pretty sure lupdate won't pick up A hacky solution might be to connect a function to the sliders' "value changed" signal and update the format externally each time it's value changes? That's probably the simplest solution that actually makes it work, but it's definitely hacky. Otherwise yeah I'd just be inclined to not handle plurals for those. |
qCeil() gives us a better translation for English (1.0 -> second, 1.1 -> seconds), but other languages probably have different rules...
…ranslation-numerus-forms
While we don't know whether some langauges use different singular/plural forms for numbers with decimal places, we can't use Qt for this anyway because tr() only accepts ints for %n.
…ranslation-numerus-forms
* .arg() doesn't handle plurals, replace %1 with %n and pass number to tr(), but only if there is an accompanying word for that we want singular/plural translations and only for whole numbers.
* Qt's translation methods only accept integers and we don't know what grammatical number form would be correct for floating-point numbers in different languages anyway.
* Add optional 2nd argument to SliderBase::SetFormat() to activate deferred plural handling:
SetFormat(QT_TRANSLATE_N_NOOP("olive::SliderBase", "%n unit(s)"), true)
* Generate en_US.ts with lupdate's -pluralonly option. Only source strings that require plural handling need to be translated (as long as the source language is American English).
992b6eb does not fix #1421. The placeholder
%nmust be used instead of%1,%2etc. and the value passed totr()instead of viaarg().In the current state, this PR breaks widgets such as the FloatSlider. When entering a new value or dragging it, the label does not update. There are also QString::arg warnings, because a former placeholder such as
%1no longer exists (replaced by%n). It will require some refactoring to properly update the UI I'm afraid, but I couldn't figure out how so far.This PR also replaces the English translation file with
lupdate . -ts app/ts/en_US.ts -pluralonly, which only contains the strings which require plural handling. I added translations for the few strings that do. As long as the source strings are in American English, then we don't need a full translation (unless we want to allow users to change the English UI?!)TODO:
Change build systemso that CMake updatesen_US.tswith-pluralonlyoption and document this special case in Wiki.-> The logic to conditionally create or add translations got removed here:
b9724e3#diff-fd99ceeacdfe207bd1168e1c30c94ed9f316895e74154c83258807c703288b3fL272-L276
qt5_add_translationis called unconditionally for updating .qm filesolive/app/CMakeLists.txt
Line 63 in c03c197
The
UPDATE_TSoption does nothing and there is no call toqt5_create_translationanywhere that would update .ts files. So there isn't anything to change to pass-pluralonly...Obsoletes #1423