Fix missing scale & chord translations

[sqrvrt]

tr() macro assumes the class name as an implicit key. With InstrumentFunctionNoteStacking.cpp, this would be lmms::InstrumentFunctionNoteStacking. The reason translations do not work on master is because QT_TRANSLATE_NOOP requires an explicit key, and the outdated class name was given (from before lmms namespace got introduced). QT_TR_NOOP does not require an explicit key, and instead fetches the class name automatically.

With this patch, only one case of QT_TRANSLATE_NOOP remains, and that is the plugin descriptors. QT_TR_NOOP might not be useful there, and the translations should be used directly with QCoreApplication::translate(key, variable). I haven't checked yet if that's the case.

(without the patch these are just untranslated strings ↓)

[regulus79]

I'm curious, why was InstrumentFunctionNoteStacking::tr() used instead of tr()? I can't find a definition of tr in InstrumentFunctionNoteStacking, so I'm confused what it is referring to.

[sqrvrt]

To my understanding, tr() is only meant to be used in the same class it's called from, and what it essentially does is it associates class/string with a translation, like so: (location appears to be irrelevant for associations themselves, only being provided for extra context)

<context>
    <name>InstrumentFunctionNoteStacking</name>
    <message>
        <location filename="../../src/core/InstrumentFunctions.cpp" line="43"/>
        <source>octave</source>
        <translation>Октава</translation>
    </message>
</context>

QT_TRANSLATE_NOOP marks the string literal for translation, but does not translate it. For some reason InstrumentFunctionNoteStacking instead of InstrumentFunctionNoteStacking::Chord was chosen, but that's what all translations use now and breaking every single translation is not my intention here.

Then tr() is called from a class that also isn't InstrumentFunctionNoteStacking (but a sub-class), and it doesn't work. Qt docs recommend using QApplication::translate() in that case, which does work so that is what I did.

note to self: .ts uses typescript highlighting, not translation

[allejok96]

I suspect this is a namespace thing. The context name should be lmms::InstrumentFunctionNoteStacking.

If you use QT_TR_NOOP instead, I think they will get the correct fully qualified name without having to manually write it out. We should investigate all usages of QT_TRANSLATE_NOOP to see if this bug exists in more places (I think .ui files are ok)

---

Side note: if you use both QT_TRANSLATE_NOOP and QCoreApplication::translate, the context is an arbitrary string.

QT_TRANSLATE_NOOP("Bah", "major");

InstrumentFunctionNoteStacking::Chord::Chord(const char* n, const ChordSemiTones& semi_tones) :
	m_name(QCoreApplication::translate("Bah", n))

[sqrvrt]

I suspect this is a namespace thing. The context name should be lmms::InstrumentFunctionNoteStacking.

That would mean nuking the appropriate translated things as far as I'm aware. You seem to be the one managing Transifex, so it's up to you. This is merely a PR with minimal possible changes to revert it back to working behavior, even if that means the behavior is not ideal.

If that's the case, should I close this PR?

[allejok96]

We don't have to worry about context changes any more, as I've turned on Transifex's translation memory fill-up feature that copies old translations when a new identical string is uploaded.

I grepped the codebase for similar errors

• Note stacking and ProjectRenderer should use QT_TR_NOOP as it's more brief and correct
• PluginDescWidget (in PluginBrowser.cpp) should call QCoreApplication::translate("PluginBrowser", ...) it's the easiest solution
• Can't get AudioDeviceSetupWidget to work, but those translations are meaningless anyway

[Capewearer]

Is the "Мажор Бипоп" the way how it should be named? Shouldn't it be "Мажорный бипоп", since "bepop" is noun, and "major" is an adjective in that particular case?

[sqrvrt]

Is the "Мажор Бипоп" the way how it should be named? Shouldn't it be "Мажорный бипоп", since "bepop" is noun, and "major" is an adjective in that particular case?

The issue is (was) translations not applying, not translations being wrong. If you want to fix individual translation strings, transifex is the right place.

[allejok96]

@sqrvrt do you want to do all the QT_TR_NOOP changes in this PR? I think it would be good to get these macros right since people will be copying them without knowing how they actually work.

[sqrvrt]

So in theory this is now ready to merge. Translation mappings need to be slightly altered, in particular because the lmms namespace got introduced (which i assume is what broke them initially, wow that was a long time ago).

HOWEVER i didn't test the ProjectRenderer one since at a glance i wasn't able to find where they were being translated at a glance. I can only assume it works the same.

---

Testing.

Example .ts diff for local testing with correct translations (prob same for ProjectRenderer):

diff --git a/data/locale/ru.ts b/data/locale/ru.ts
index 13373d1df..3bd8d852f 100644
--- a/data/locale/ru.ts
+++ b/data/locale/ru.ts
@@ -267,7 +267,7 @@ Simple88 (2016)</translation>
     </message>
 </context>
 <context>
-    <name>InstrumentFunctionNoteStacking</name>
+    <name>lmms::InstrumentFunctionNoteStacking</name>
     <message>
         <location filename="../../src/core/InstrumentFunctions.cpp" line="43"/>
         <source>octave</source>

Then, in order to compile the locale run /usr/lib64/qt5/bin/lrelease /path/to/lmms/data/locale/ru.ts (at least for me lrelease wasn't in PATH). Then simply launch lmms binary as normal (no recompilation needed).