D12982: Make the new KCMs with QtQuick translatable

[Yuri Chornoivan]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:738da689cbd2: Make the new KCMs with QtQuick translatable 
(authored by yurchor).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D12982?vs=34932=35229#toc

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12982?vs=34932=35229

REVISION DETAIL
  https://phabricator.kde.org/D12982

AFFECTED FILES
  kcms/icons/main.cpp
  kcms/launch/CMakeLists.txt
  kcms/launch/Messages.sh
  kcms/translations/CMakeLists.txt
  kcms/translations/Messages.sh
  kcms/workspaceoptions/CMakeLists.txt
  kcms/workspaceoptions/Messages.sh

To: yurchor, #plasma, kde-i18n-doc, ltoscano, davidedmundson
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[David Edmundson]

davidedmundson added a comment.


  > That's different: I see KCMs are libraries,
  
  
  
  > I think it would be acceptable to use 
KLocalizedString::setApplicationDomain, even if introduces an exception, but at 
least not *another* way to set the translation domain.
  
  I don't think so for two reasons. Firstly it's a single domain and we are 
plugins within another shell.
  
  Secondly, from QML our i18n calls go to klocalizedcontext which if a domain 
is set will explicitly use it. So in order to use
  
  KLocalizedString::setApplicationDomain we have to completely remove our
  
  d->_qmlObject->setTranslationDomain(aboutData()->componentName()); in 
configmodule.cpp
  
  I don't think that's viable, especially with the staggered release cycles.
  
  -
  
  I'd be happy to introduce a ConfigModule::setTranslationDomain in 
kdeclarative. 
  Which would allow doing everything from code without having to make names 
match.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano, davidedmundson
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Luigi Toscano]

ltoscano added a comment.


  In D12982#270846 , @davidedmundson 
wrote:
  
  > > is this something that needs to be fixed in kdeclarative
  >
  > I don't think so.
  >  This patch seems fine, it's no different to our current state of how 
applet translations work following a fixed pot schema.
  
  
  That's different: I see KCMs are libraries, but they introduce an exception 
in the way the translation domain is defined for a library. And even for 
applets I would argue that the case may be needed, but let's move it.
  
  Applet won't need to co-exist, KCMs may need to do it.
  
  > 
  > 
  >>   would a patch to fix this (define the translation domain in the same way 
as the other libraries, not related to component name) accepted?
  > 
  > Accepted, sure. But the QtQuick loading is quite separate from the library 
here, so I'm not sure it's easy.
  
  I think it would be acceptable to use KLocalizedString::setApplicationDomain, 
even if introduces an exception, but at least not *another* way to set the 
translation domain.
  
  Would that work/be easier to implement for this dynamic setting?
  
  > If we want to have QtQuick only KCMs (something I think probably isn't 
useful, but I know Marco wants) we would still need to load pots based on 
plugin metadata names like it is currently.
  
  I still think that there is a use case for translation domain unrelated to 
metadata (if you mean static KAboutData settings).

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano, davidedmundson
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Marco Martin]

mart added a comment.


  In D12982#270846 , @davidedmundson 
wrote:
  
  > Accepted, sure. But the QtQuick loading is quite separate from the library 
here, so I'm not sure it's easy.
  >
  > If we want to have QtQuick only KCMs (something I think probably isn't 
useful, but I know Marco wants) we would still need to load pots based on 
plugin metadata names like it is currently.
  
  
  qtquick only kcms are actually supported by plasma-settings the one used in 
plasma mobile, but i don't think they are necessarly a good idea
  on the other hand, one thing that i was looking into, was to use the metadata 
to generate a default kaboutdata and not be forced to have it manually in every 
kcm as now.. tough it would cause a problem regarding to this

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano, davidedmundson
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[David Edmundson]

davidedmundson added a comment.


  > is this something that needs to be fixed in kdeclarative
  
  I don't think so.
  This patch seems fine, it's no different to our current state of how applet 
translations work following a fixed pot schema.
  
  >   would a patch to fix this (define the translation domain in the same way 
as the other libraries, not related to component name) accepted?
  
  Accepted, sure. But the QtQuick loading is quite separate from the library 
here, so I'm not sure it's easy.
  
  If we want to have QtQuick only KCMs (something I think probably isn't 
useful, but I know Marco wants) we would still need to load pots based on 
plugin metadata names like it is currently.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano, davidedmundson
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Luigi Toscano]

ltoscano added a comment.


  I'd really like to have a clear answer to some questions:
  
  - is this something that needs to be fixed in kdeclarative, which makes it 
impossible to have a proper fix it in 5.13?
  - would a patch to fix this (define the translation domain in the same way as 
the other libraries, not related to component name) accepted?
  
  The answer to the first question will make the next move. If the only fix is 
in kdeclarative, then Yuri please revert to the initial version. In that case 
it would be the only possible short-term fix even I don't like it.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano, davidedmundson
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Yuri Chornoivan]

yurchor updated this revision to Diff 34932.
yurchor added a comment.


  Normalized to Luigi's scheme. May not work. Please test at least for English 
(default).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12982?vs=34475=34932

REVISION DETAIL
  https://phabricator.kde.org/D12982

AFFECTED FILES
  kcms/icons/main.cpp
  kcms/launch/CMakeLists.txt
  kcms/launch/Messages.sh
  kcms/launch/launchfeedback.cpp
  kcms/translations/CMakeLists.txt
  kcms/translations/Messages.sh
  kcms/translations/translations.cpp
  kcms/workspaceoptions/workspaceoptions.cpp

To: yurchor, #plasma, kde-i18n-doc, ltoscano, davidedmundson
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Luigi Toscano]

ltoscano added a comment.


  Should I create a bug/task somewhere to track the work needed to fix this 
properly, where we can discuss about the proper solution? (fix in kdeclarative?)

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano, davidedmundson
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.


  > I think that this patch would be the correct way to fix 5.13.
  
  From Luigi is good enough for me.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano, davidedmundson
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Yuri Chornoivan]

yurchor added a comment.


  I have no idea what to do with all the suggestions and cannot test the 
required changes. For sure, I can formally change *.cpp, CMakeLists.txt and 
Messages.sh, but it can break the KCMs to the state that they will not be able 
to function even in English.
  
  May it be that someone will prepare the proper patch which takes into account 
Luigi's requirements?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Luigi Toscano]

ltoscano added a comment.


  In D12982#267013 , @davidedmundson 
wrote:
  
  > > Please also note that, if supporting a different name for the translation 
catalog would mean changing something in Frameworks,...
  >
  > Relevant parts are:
  >
  > .pot needs to match TRANSLATION_DOMAIN for C++ code
  
  
  .pot needs to match TRANSLATION_DOMAIN for libraries; otherwise you set the 
domain through KLocalizedString::setApplicationDomain.
  
  > .pot needs to match aboutData()->componentName() for QtQuick loaded code  
(configmodule.cpp:159)
  > 
  > Neither of those should require a frameworks change as they can all be 
adjusted
  
  configmodule.cpp is part of KDeclarative, so making sure that you can use 
TRANSLATION_DOMAIN requires a change in Frameworks.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[David Edmundson]

davidedmundson added a comment.


  > Please also note that, if supporting a different name for the translation 
catalog would mean changing something in Frameworks,...
  
  Relevant parts are:
  
  .pot needs to match TRANSLATION_DOMAIN for C++ code
  .pot needs to match aboutData()->componentName() for QtQuick loaded code  
(configmodule.cpp:159)
  
  Neither of those should require a frameworks change as they can all be 
adjusted
  
  From my POV, this patch looks fine.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: davidedmundson, mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Luigi Toscano]

ltoscano added a comment.


  Please also note that, if supporting a different name for the translation 
catalog would mean changing something in Frameworks, I think that this patch 
would be the correct way to fix 5.13. Nevertheless this use case should be 
fixed afterwards so that 5.14 could make use of it.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Luigi Toscano]

ltoscano added a comment.


  Libs connected to KCMs or other libraries part of Plasma? About the latter, I 
don't recall relevant case of renamings/reshuffling, so probably no need.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Marco Martin]

mart added a comment.


  In D12982#266967 , @ltoscano wrote:
  
  >   We already had changes in the structure of kcm, and I prefer the names to 
be future-proof.
  >   
  >
  > Besides, we already have some versioned names, so this use case should be 
supported.
  
  
  that's fine.. but would that mean we have to rename the libs too?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: mart, hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12982: Make the new KCMs with QtQuick translatable

[Luigi Toscano]

ltoscano added a comment.


  We already had changes in the structure of kcm, and I prefer the names to be 
future-proof.
  
  Besides, we already have some versioned names, so this use case should be 
supported.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D12982: Make the new KCMs with QtQuick translatable

[Eike Hein]

hein added a comment.


  What's the point of "kcm5"? kcm_translations.pot matches the package and .so 
names, which is nice and neat. Why does the pot need to be more versioned, is 
there a namespace conflict with KDE4?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: hein, aacid, ltoscano, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D12982: Make the new KCMs with QtQuick translatable

[Albert Astals Cid]

aacid added a comment.


  Plasma people any hint of what are your preferences on fixing this?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: aacid, ltoscano, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D12982: Make the new KCMs with QtQuick translatable

[Luigi Toscano]

ltoscano added a comment.


  I expect the plasma developer to be able to help with fixing this.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: ltoscano, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12982: Make the new KCMs with QtQuick translatable

[Yuri Chornoivan]

yurchor added a comment.


  In D12982#265059 , @ltoscano wrote:
  
  > I don't want to go back from kcm5_workspace to kcm_workspace. Any solution 
should allow to define a specific name for the KCMs.
  
  
  This will probably need some refactoring inside and renaming several .json 
files outside. I have no competence to do it right without testing and have no 
testing capabilities until the next week. Is it necessary?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: ltoscano, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12982: Make the new KCMs with QtQuick translatable

[Luigi Toscano]

ltoscano requested changes to this revision.
ltoscano added a comment.
This revision now requires changes to proceed.


  I don't want to go back from kcm5_workspace to kcm_workspace. Any solution 
should allow to define a specific name for the KCMs.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

To: yurchor, #plasma, kde-i18n-doc, ltoscano
Cc: ltoscano, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12982: Make the new KCMs with QtQuick translatable

[Yuri Chornoivan]

yurchor created this revision.
yurchor added reviewers: Plasma, kde-i18n-doc.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
yurchor requested review of this revision.

REVISION SUMMARY
  According to this comment:
  
  https://marc.info/?l=kde-i18n-doc=152672268226040=2
  
  TRANSLATION_DOMAIN in CMakeLists.tx is not enough to make QML code 
translatable.
  
  This can be easily tested using Neon:
  
  https://files.kde.org/neon/images/neon-devedition-gitunstable/current/
  
  This revision is trying to mitigate this issue with minimal changes in the 
code.

TEST PLAN
  1. Run Neon with some ~100% translation package (sv or uk).
  
  2. Rename translation catalogs as follows:
  
  kcmlaunchfeedback -> kcm_launchfeedback
  kcmtranslations -> kcm_translations
  kcm5_workspace -> kcm_workspace
  
  3. Test if icons, launchfeedback, translations, and workspaceoptions modules 
are translated.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D12982

AFFECTED FILES
  kcms/icons/main.cpp
  kcms/launch/CMakeLists.txt
  kcms/launch/Messages.sh
  kcms/translations/CMakeLists.txt
  kcms/translations/Messages.sh
  kcms/workspaceoptions/CMakeLists.txt
  kcms/workspaceoptions/Messages.sh

To: yurchor, #plasma, kde-i18n-doc
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart