Fix broken localization of kcmkwinrules
AbandonedPublic

Authored by yurchor on Apr 29 2020, 5:49 AM.

Details

Reviewers
broulik
Group Reviewers
KWin
Summary

Current extraction and applying translations are broken.

Patch mostly by Victor Ryzhykh.

Test Plan

Run extract-messages.sh manually, test in vivo.

Diff Detail

Repository
R108 KWin
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26106
Build 26124: arc lint + arc unit
yurchor created this revision.Apr 29 2020, 5:49 AM
Restricted Application added a project: KWin. · View Herald TranscriptApr 29 2020, 5:49 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
yurchor requested review of this revision.Apr 29 2020, 5:49 AM
yurchor edited the summary of this revision. (Show Details)Apr 29 2020, 5:51 AM
zzag added a subscriber: zzag.Apr 29 2020, 6:05 AM
zzag added inline comments.
kcmkwin/kwinrules/Messages.sh
2

If we rename the pot file to kcm_kwinrules.pot and adjust TRANSLATION_DOMAIN in CMakeLists.txt, will we still need to use i18nd?

yurchor added inline comments.
kcmkwin/kwinrules/Messages.sh
2

@ltoscano Can you clear up the current rules of catalog naming for this case? Thanks in advance for your help.

pino added a subscriber: pino.Apr 29 2020, 6:26 AM
pino added inline comments.
kcmkwin/kwinrules/Messages.sh
2

TRANSLATION_DOMAIN is a C/C++ define, it does not apply to strings in QML files.

zzag added inline comments.Apr 29 2020, 6:28 AM
kcmkwin/kwinrules/Messages.sh
2

Could someone explain why we don't need to use i18nd in Destop Effects or Virtual Desktops KCM?

broulik requested changes to this revision.EditedApr 29 2020, 6:37 AM
broulik added a subscriber: broulik.

I bet the KAboutData in the KCM c++ is wrong. The component name determines the translation domain to be used on the KCM QML side.

This revision now requires changes to proceed.Apr 29 2020, 6:37 AM

I bet the KAboutData in the KCM c++ is wrong. The component name determines the translation domain to be used on the KCM QML side.

Some time ago I asked if it was a way to have the QML-based KCMs use the same way for setting the translation domain as the C++ ones. This is also needed for all KCMs defined outside Plasma, because technically they should be called kcm5_<foo> to prevent co-installability issues (with kdelibs 4.x back then, with the KF6 ones in a few months). Would it be possible to revisit this point? We are going to have installation conflicts when KF6 is out otherwise.

Would it be possible to revisit this point? We are going to have installation conflicts when KF6 is out otherwise.

Feel free to add a task to the allmighty KF6 board https://phabricator.kde.org/tag/kf6/

yurchor updated this revision to Diff 81488.Apr 29 2020, 8:09 AM

Change KAboutData to be in sync with the translation catalog name

Would it be possible to revisit this point? We are going to have installation conflicts when KF6 is out otherwise.

Feel free to add a task to the allmighty KF6 board https://phabricator.kde.org/tag/kf6/

Added T13056, even though it is not KF6-specific (but I guess it would be easier to implement it directly there).

Set the value to "auto about = new KAboutData(QStringLiteral("kcmkwinrules"),"
Got such an error.

yurchor updated this revision to Diff 81544.Apr 29 2020, 7:18 PM

Move i18nd() back

yurchor updated this revision to Diff 81545.Apr 29 2020, 7:20 PM

Replace KAboutData with an old value

victorr added a comment.EditedApr 29 2020, 7:21 PM

If you rename it to “kcm_kwinrules” with the attached patch, then the translation works.
Now my packages are going to check.

If you rename it to “kcm_kwinrules” with the attached patch, then the translation works.
Now my packages are going to check.

Yes, renaming to “kcm_kwinrules” corrects the display of the translation.

victorr added a comment.EditedApr 30 2020, 1:28 AM

If you rename it to “kcm_kwinrules” with the attached patch, then the translation works.
Now my packages are going to check.

Yes, renaming to “kcm_kwinrules” corrects the display of the translation.

No, not everywhere translation works.
It took one more patch to the previous one.


Before

After

So what is the resolution? What should I do with this?

Thanks in advance for your advice.

davidedmundson added inline comments.
kcmkwin/kwinrules/Messages.sh
2

It all comes from

configmodule.cpp in kdeclarative

QQuickItem *ConfigModule::mainUi()

d->_qmlObject->setTranslationDomain(aboutData()->componentName());

If this is set then anything loaded within this QML Context will have the right domain.
We do use ConfigModule in this case.

kcmrules.cpp: auto about = new KAboutData(QStringLiteral("kcm_kwinrules"),

So as Vlad says from what I can tell if this pot is renamed it should magically start working without i18nd.

broulik requested changes to this revision.May 4 2020, 12:20 PM

KAboutData is used for translation context and for finding the plug-in. Make sure to update the install location in kpackage5/ in CMake when changing it

This revision now requires changes to proceed.May 4 2020, 12:20 PM
yurchor abandoned this revision.May 4 2020, 1:12 PM

This cannot be easily merged anymore (files were changed meanwhile) so I prefer to abandon this revision.

@broulik Can you fix it if you know how to do this please?

This cannot be easily merged anymore (files were changed meanwhile) so I prefer to abandon this revision.

Maybe I should make a new patch?
Today I planned to build kwin with this commit https://cgit.kde.org/kwin.git/commit/?id=1a3cb256d74166ba175f4634904abea84928b635
Although you can wait with this.:)

This cannot be easily merged anymore (files were changed meanwhile) so I prefer to abandon this revision.

Maybe I should make a new patch?
Today I planned to build kwin with this commit https://cgit.kde.org/kwin.git/commit/?id=1a3cb256d74166ba175f4634904abea84928b635
Although you can wait with this.:)

Whatever you want.

Personally, I think that the whole system is wrong. Every single release someone break the translation of KCM, there is no conventional naming, conventional style, and written rules. Just a prediction: the next release of Plasma will have broken KCMs too.

This cannot be easily merged anymore (files were changed meanwhile) so I prefer to abandon this revision.

Maybe I should make a new patch?
Today I planned to build kwin with this commit https://cgit.kde.org/kwin.git/commit/?id=1a3cb256d74166ba175f4634904abea84928b635
Although you can wait with this.:)

Whatever you want.

Made a new patch.

Made a new patch.

It will not be accepted because it uses i18nd(). @broulik wants this problem to be resolved on KAboutData/CMakeLists.txt level.

victorr added a comment.EditedMay 4 2020, 2:10 PM

Made a new patch.

It will not be accepted because it uses i18nd(). @broulik wants this problem to be resolved on KAboutData/CMakeLists.txt level.

It would be nice.
And when I tried this option, this error got out

Just please patch Messages.sh to rename the file, I will take care of moving it.

Regarding the more general issue of the name of the template, as I mentioned earlier, I created T13056 . It would be good if it was fixed before KF6 though.

It would be nice.
And when I tried this option, this error got out

Because you need to fix the damn location where the package is installed

It would be nice.
And when I tried this option, this error got out

Because you need to fix the damn location where the package is installed

I will not create additional differential revisions. I do not know what "kcmkwinrules" should be left and what should be replaced with "kcm_kwinrules" and cannot test by myself.

Dear Plasma developers, can you resolve this problem without incompetent translators?

It would be nice.
And when I tried this option, this error got out

Because you need to fix the damn location where the package is installed

I did with the patch from this post
https://phabricator.kde.org/D29270#659987
At first I was pleased that everything was translated.
Then I saw an error in setting the window title, and made another patch, which is here https://phabricator.kde.org/D29270#660131