Download and use localization files for KStars Lite
AbandonedPublic

Authored by ckertesz on Jul 26 2018, 7:45 PM.

Details

Reviewers
mutlaqja
pino
apol

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
ckertesz requested review of this revision.Jul 26 2018, 7:45 PM
ckertesz created this revision.
ckertesz added a reviewer: mutlaqja.
ckertesz set the repository for this revision to R321 KStars.
Restricted Application added a project: KDE Edu. · View Herald TranscriptJul 26 2018, 7:45 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
pino requested changes to this revision.Jul 26 2018, 8:04 PM
pino added a subscriber: pino.

This does not look good for different reasons:

  • the KI18n framework is marked as working on Android (and looking at its git history, there were fixes for it); hence, if it still does not work, please fix it instead (so it will work for kstars any any other KI18n-based application)
  • if you really need to define i18n() as gettext(), then wrap it as QString::fromUtf8(), otherwise the encoding is completely wrong (implicit char* -> QString cast uses latin1/ascii)
  • it mixes unrelated changes (like header reshuffling) -- please make sure each commit is not an aggregate of many unrelated changes!
  • all the various string changes, like the introduction of string puzzles, are big no-no

Please get KI18n to work: all these changes are intrusive, and wrong, workarounds to that.

This revision now requires changes to proceed.Jul 26 2018, 8:04 PM
aacid added a subscriber: aacid.Jul 26 2018, 9:09 PM
In D14411#298786, @pino wrote:

This does not look good for different reasons:

  • the KI18n framework is marked as working on Android (and looking at its git history, there were fixes for it); hence, if it still does not work, please fix it instead (so it will work for kstars any any other KI18n-based application)

It does work, i've used it.

In D14411#298786, @pino wrote:

This does not look good for different reasons:

  • the KI18n framework is marked as working on Android (and looking at its git history, there were fixes for it); hence, if it still does not work, please fix it instead (so it will work for kstars any any other KI18n-based application)

It does work, i've used it.

Where? Do you have any code online? My Google search did not give any results. I asked Aleix Pol some days ago and he could not reference any KDE app which uses KI18n with gettext-style translations on Android.

aacid added a comment.Jul 26 2018, 9:45 PM
In D14411#298786, @pino wrote:

This does not look good for different reasons:

  • the KI18n framework is marked as working on Android (and looking at its git history, there were fixes for it); hence, if it still does not work, please fix it instead (so it will work for kstars any any other KI18n-based application)

It does work, i've used it.

Where? Do you have any code online? My Google search did not give any results. I asked Aleix Pol some days ago and he could not reference any KDE app which uses KI18n with gettext-style translations on Android.

Actually i was wrong, in KTuberling i get i18n'ed content but it comes from KConfig .ini files and not from a .mo

But we should really debug what's missing in ki18n if it doesn't work for you, i don't see any reason why it wouldn't work.

Thanks folks for your input. We really need to get localizations done for KStars on Android. Despite that ki18n for Android is marked as "working", it is not. I think Csaba can detail the reasons for this but I'm not sure what it would take to get them fixed. These changes are part of KStars GSoC 2018 project so we're limited by scope and time.

apol requested changes to this revision.Jul 27 2018, 1:49 PM
apol added a subscriber: apol.

Please no. Don't ifdef our framework out. This is absurd.
Let's fix it once and have it work.

If libintl-lite doesn't do the job, let's replace it with one of the alternatives, but let's not just workaround ourselves.

CMakeLists.txt
17

When you build kstars lite you need to fetch translations?

I think it's better if we pass -DKDE_L10N_AUTO_TRANSLATIONS=ON when we need to fetch translations.

datahandlers/catalogdb.cpp
24

Why do you do the define thing and replace it anyway?

814

This QString constructor is wrong here, i18n returns a QString.

kstars/auxiliary/colorscheme.cpp
113

It's a warning, maybe not really worth translating ever?

apol added a comment.Jul 27 2018, 1:50 PM

Thanks folks for your input. We really need to get localizations done for KStars on Android. Despite that ki18n for Android is marked as "working", it is not. I think Csaba can detail the reasons for this but I'm not sure what it would take to get them fixed. These changes are part of KStars GSoC 2018 project so we're limited by scope and time.

This is not how GSoC works, we don't have to merge half-baked stuff because we found a problem. If there's a problem we figure it out. @ckertesz can have a successful GSoC without spoiling kstars codebase.

pino added inline comments.Jul 28 2018, 8:15 AM
datahandlers/catalogdb.cpp
606

Nice, so many string puzzles in this file... will fix them.

kstars/auxiliary/colorscheme.cpp
113

Yup, will fix it.

kstars/indi/clientmanagerlite.cpp
242

These changes are basically no-nops. Also, more string puzzles to fix...

kstars/kstarslite/qml/dialogs/helpers/LocationEdit.qml
424

This almost looked like a good change... if not for the switch from xi18n to i18n. Anyway, will fix this string puzzle too...

kstars/kstarslite/qml/modules/helpers/TimeSpinBox.qml
169

All the handling of units in the existing QML source is already wrong...

ckertesz abandoned this revision.Aug 2 2018, 10:09 PM