Fix plugin-metadata translations on windows
Needs ReviewPublic

Authored by sars on Nov 28 2019, 5:36 PM.

Details

Reviewers
aacid
vonreth
Summary

On Windows, the plugin metadata always follows the current region setting and any language set for the application is ignored.

The reason seems to be that QLocale is not changed even if the LANGUAGE env is set. To work around this we can read the LANGUAGE env variable and use that if it is set in stead of QLocale::name().

I'm not 100% sure if this is the right solution, but this at least makes the plugin configuration page in Kate use the same language as the rest of the application if the language is manually sett.

Test Plan

Open the plugin configuration page in Kate on Windows and see that the plugin information uses the same language as the rest of the application.

Diff Detail

Repository
R244 KCoreAddons
Lint
Lint Skipped
Unit
Unit Tests Skipped
sars created this revision.Nov 28 2019, 5:36 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 28 2019, 5:36 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
sars requested review of this revision.Nov 28 2019, 5:36 PM

The doc doesn't mention the LANGUAGE var.

QLocale::QLocale()

Constructs a QLocale object initialized with the default locale. If no default locale was set using setDefault(), this locale will be the same as the one returned by system().

See also setDefault().
void QLocale::setDefault(const QLocale &locale)

Sets the global default locale to locale. These values are used when a QLocale object is constructed with no arguments. If this function is not called, the system's locale is used.

Warning: In a multithreaded application, the default locale should be set at application startup, before any non-GUI threads are created.

Warning: This function is not reentrant.

See also system() and c().
aacid added a comment.Dec 1 2019, 4:37 PM

Right, but we set it in initializeLanguages.

https://github.com/KDE/kxmlgui/blob/master/src/kswitchlanguagedialog_p.cpp#L81

I think the bug is that initializeLanguages should set the proper stuff in QLocale for windows too.

I had this that fixes part of a similar problem https://phabricator.kde.org/D23119 can you check if that helps at all on windows?

Why can't we use setDefault?
Your linked patch does not look like it would change the behaviour on Windows?

aacid added a comment.Dec 1 2019, 7:01 PM

To be honest i don't remember why we don't use setDefault.

It's a solution we could investigate

sars added a comment.Dec 1 2019, 7:19 PM

ki18n does not seem to use QLocale to get the language for translation, but uses LANGUAGE env. Also gettext uses the env variable.

Well it sounds to me like we might need both.
But the issue this pr tries to tackle would probably be gone with setDefault

aacid added a comment.Dec 1 2019, 9:40 PM
In D25599#570372, @sars wrote:

ki18n does not seem to use QLocale to get the language for translation

It already does that on non unix

appendLanguagesFromQLocale(localeLanguages, QLocale::system());

the other big question is why it's using QLocale::system and not QLocale() itself

aacid added a comment.Dec 2 2019, 10:07 PM

Sure about what?

I don't know what that link is supposed to mean :)

Sure about what?

I don't know what that link is supposed to mean :)

I don't see a call to QLocale::setDefault so I don't see how QLocale() should yield a different result.

aacid added a comment.Dec 3 2019, 10:25 PM

Sure about what?

I don't know what that link is supposed to mean :)

I don't see a call to QLocale::setDefault so I don't see how QLocale() should yield a different result.

Right i never said that, did I?

sars added a comment.Dec 4 2019, 8:24 AM

So the conclusion is that we are not using setDefault() yet and ki18n & gettext uses the LANGUAGE env so the logical thing, for now, is to also use LANGUAGE env here in KCoreAddons for the plugin metadata translations?

In D25599#571880, @sars wrote:

So the conclusion is that we are not using setDefault() yet and ki18n & gettext uses the LANGUAGE env so the logical thing, for now, is to also use LANGUAGE env here in KCoreAddons for the plugin metadata translations?

For me it sounds completely wrong to use an env var here, could you instead try weather a setDefault at the correct place solves the issue?

sars added a comment.Dec 4 2019, 12:45 PM

I would agree that a UNIX/POSIX env var is not the most logical thing to use on Windows, but how do we ensure that we are in sync with ki18n? The usage of QLocale in ki18n looks like it is just adding extra languages to a list of languages, not actually using QLocale to determine what catalog to load. At least LANGUAGE overrides any QLocale settings. Should ki18n not be moved to using QLocale and setDefault()?

Do you BTW have a hint at what the "correct" place is to use QLocale::setDefault()?

aacid added a comment.Dec 4 2019, 9:37 PM
In D25599#572013, @sars wrote:

I would agree that a UNIX/POSIX env var is not the most logical thing to use on Windows, but how do we ensure that we are in sync with ki18n? The usage of QLocale in ki18n looks like it is just adding extra languages to a list of languages, not actually using QLocale to determine what catalog to load. At least LANGUAGE overrides any QLocale settings. Should ki18n not be moved to using QLocale and setDefault()?

Do you BTW have a hint at what the "correct" place is to use QLocale::setDefault()?

https://github.com/KDE/kxmlgui/blob/master/src/kswitchlanguagedialog_p.cpp#L81

sars added a comment.Dec 7 2019, 8:55 AM

Yes it works :)

(I added QLocale::setDefault(QLocale(QString::fromLatin1(languageCode))); to the end of the if statement in initializeLanguages())

Hmm... For an application like Kate it is totally fine to set the language through kxmlgui, but kxmlgui is tier 3 while kcoreaddons and ki18n are tier1. Sure any application can use setDefault() to change the language of the plugin metadata, but that does not change the translation language in ki18n.

I think the proper solution would be to use the same mechanism in ki18n and KCoreAddons. If we use the env variable we need to change KCoreAddons and if we want to use QLocale we need to change KI18n to use QLocale.

aacid added a comment.Dec 8 2019, 10:31 PM
In D25599#573581, @sars wrote:

Yes it works :)

(I added QLocale::setDefault(QLocale(QString::fromLatin1(languageCode))); to the end of the if statement in initializeLanguages())

Hmm... For an application like Kate it is totally fine to set the language through kxmlgui, but kxmlgui is tier 3 while kcoreaddons and ki18n are tier1. Sure any application can use setDefault() to change the language of the plugin metadata, but that does not change the translation language in ki18n.

I think the proper solution would be to use the same mechanism in ki18n and KCoreAddons. If we use the env variable we need to change KCoreAddons and if we want to use QLocale we need to change KI18n to use QLocale.

Proper solution for what? I don't understand what you're trying to fix. I thought your problem was "when people use kxmlgui to set a custom language for the app things don't work", but now you're saying of not using kxmlgui so i'm confused.