Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use of translation functions #5629

Merged
merged 1 commit into from
Aug 30, 2020
Merged

Conversation

DomClark
Copy link
Member

Fixes some outdated or incorrect usage of translation functions.

  • The pluginBrowser class has since been renamed to PluginBrowser, so translations using that context have been updated to use the new name.
  • The context for audio backend name translations has been changed from setupWidget to AudioDeviceSetupWidget. This is consistent with the use of MidiSetupWidget for MIDI backends.
  • VeSTige and VstEffect no longer pass strings consisting of a single integer id through tr.
  • The line presName.replace(tr("\""), tr("'")); // QFileDialog unable to handle double quotes properly from vst_base has been changed not to use tr - it doesn't make sense to allow translators to override this and break QFileDialog.
  • Static strings in InstrumentSoundShaping now use QT_TRANSLATE_NOOP rather than tr; tr has no effect there since the translations have not been loaded yet at that point.
  • Some places in TrackContentObjectView, SetupDialog, and SampleTCOView had calls to tr factored out of ternary expressions, or into lambdas. This breaks lupdate, which can't recognise anything but string literals within tr calls. Calls to tr have been moved to wrap the literals.
  • In cases where it is necessary to pass the contents of a QString to tr, toLatin1 has been changed to toUtf8, since this is the encoding Qt expects.
  • PluginBrowser now translates descriptions in its tooltips for plugins that aren't subplugins.
  • An unused static translation has been removed from InstrumentTrack.

@DomClark DomClark requested a review from liushuyu August 12, 2020 22:31
@LmmsBot
Copy link

LmmsBot commented Aug 12, 2020

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

Windows

macOS

🤖
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://8259-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.689-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8259?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://8257-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.689-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8257?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://8258-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.689-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8258?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/mxbk7bgay6bgocd0/artifacts/build/lmms-1.2.2-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/34628258"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/14gekdrrfndoa52b/artifacts/build/lmms-1.2.2-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/34628258"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://8260-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.689-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8260?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "f34f894a84f9885b91da295f8ee676fb96b3c1e1"}

Copy link
Member

@liushuyu liushuyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM.

I will update the translation template after this PR gets merged. Thanks for your work!

setToolTip(_pk.description());
setToolTip(_pk.desc->subPluginFeatures
? _pk.description()
: tr(_pk.desc->description));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this will certainly not break anything, I am not sure whether this will be effective since _pk.desc->description is dynamic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case that _pk.desc->subPluginFeatures is null, _pk.desc is a static descriptor from a plugin binary, e.g.:

Plugin::Descriptor PLUGIN_EXPORT amplifier_plugin_descriptor =
{
STRINGIFY( PLUGIN_NAME ),
"Amplifier",
QT_TRANSLATE_NOOP( "PluginBrowser", "A native amplifier plugin" ),
"Vesa Kivimäki <contact/dot/diizy/at/nbl/dot/fi>",
0x0100,
Plugin::Effect,
new PluginPixmapLoader("logo"),
NULL,
NULL
} ;

I'm not aware of any code that modifies these (we should probably make them const), and the description is clearly intended to be translated. This also matches the code in EffectSelectDialog:

labelText += "<p><b>" + tr("Description") + ":</b> " + qApp->translate( "PluginBrowser", descriptor.description ) + "</p>";

@DomClark
Copy link
Member Author

Merge?

@liushuyu
Copy link
Member

Merge?

I am very sorry for the late response. I think it's good to merge now.

@DomClark DomClark merged commit 5efb3a1 into LMMS:master Aug 30, 2020
@DomClark DomClark deleted the fix-translations branch March 12, 2021 19:06
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants