This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122680/

On March 2nd, 2015, 9:06 a.m. CET, Martin Gräßlin wrote:

Ship It!

On March 2nd, 2015, 9:39 a.m. CET, Luigi Toscano wrote:

Please fix the extracted pot so that it is called <file>_qt.pot.

(personally speaking, I would strongly suggest lxde to use ki18n, but that's another discussion)

On March 2nd, 2015, 9:44 a.m. CET, Luigi Toscano wrote:

Sorry, there is also another change about the script used, check other tier1 frameworks

On March 2nd, 2015, 9:51 a.m. CET, Jerome Leclanche wrote:

I don't understand what you're asking of me - in the last diff, I'm not touching the pot file at all.

On March 2nd, 2015, 9:54 a.m. CET, Luigi Toscano wrote:

Sorry again, it' s a bit.more complicated as there is already such _qt.pot file (the main one). So probably it should be extended to extract also runtime translations and the appropriate changes to translations domain applied here. Please cc i18n, I can't do a complete check now.

On March 2nd, 2015, 9:58 a.m. CET, Luigi Toscano wrote:

wrt why I'm asking: the Messages.sh, maybe the top-level one and few cmake calls to be changed to have translations working. Plase add i18n group.

On March 2nd, 2015, 12:23 p.m. CET, Hrvoje Senjan wrote:

i'd like to mention that translations for the daemon aren't working anyway with 5.7.0 tar, as they aren't wrapped with ki18n macro, but with ECMPoQmTools

On March 2nd, 2015, 12:33 p.m. CET, Albert Astals Cid wrote:

@Hrvoje, the daemon is using i18n just fine in 5.7.0, ecmpoqmtools is for the library which uses .po.

@Jerome, yes this change is incomplete, you either need to remove the Messages.sh in the daemon and add the ecm loading of the existing kglobalaccel5_qt qm to the daemon or create two different catalogs, i'd say having just one probably makes sense. You won't need -DTRANSLATION_DOMAIN anymore since that's for ki18n

On March 2nd, 2015, 12:37 p.m. CET, Hrvoje Senjan wrote:

afaics, ki18n only knowns about mo files, while all translations in kglobalaccel 5.7 are installed as qm files.. it is also possible i missunderstood something ;-)

On March 2nd, 2015, 8:01 p.m. CET, Albert Astals Cid wrote:

You're right, the cmake code is wrong and is installing kglobalaccel5.po wrongly as kglobalaccel5.qm I guess noone expected .qm and non .qm files to coexist in a single tarball.

Could we perhaps ship this as is and then have someone from the i18n team fix the translations properly? I mean Jerome is working on lxqt and we have much better understanding of our i18n stuff in frameworks, so couldn't we help out fixing that issue afterwards?

Additionally, is the removing of Messages.sh and adding ECMPoQmTools all that is needed? I could even do that.

- Martin

On March 2nd, 2015, 10 a.m. CET, Jerome Leclanche wrote:

Review request for KDE Frameworks, Localization and Translation (l10n), Martin Gräßlin, and Martin Klapetek.

By Jerome Leclanche.

Updated March 2, 2015, 10 a.m.

Repository: kglobalaccel

Description

Remove the runtime's KAboutData
    
The about data was unexposed, but created a dependency on KCoreAddons (for
KAboutData) and in turn on KI18n for the translations of the aboutData.
    
This removes both dependencies as well as the string extraction scripts.

--

Author notes: This is a RFC. We don't use kglobalaccel in LXQt but we would
like to, however it currently has too many dependencies. See
https://github.com/lxde/lxqt/issues/507 for related discussion.
I'm unsure myself if the about data is actually exposed somewhere I completely
missed, but it doesn't look that way.

Testing

Compiles and runs. No further testing done.

Diffs

CMakeLists.txt (68ad795)
src/runtime/CMakeLists.txt (e639fa5)
src/runtime/globalshortcutsregistry.cpp (3e4d720)
src/runtime/kglobalacceld.cpp (4e7cb9d)
src/runtime/main.cpp (fdf4d62)

View Diff