Auto-detect language

[Cryspia]

If the user does not have the freeseer.conf file, the function will detect the system location for default language. If there is a config file, the language setting in that file will have higher priority. For Chinese, If the location is not zh_CN, Traditional Chinese will be used. If there is no translation for the system location, English will be used.

Fixes #476

[Cryspia]

It is so hard to pass the build :(

[mtomwing]

Can you explain your logic for figuring out the system language? I can't really follow your code.

[Cryspia]

I did not add code for getting the system language (The system language detecting is already in FreeseerApp.py, but the result was not really been used)

[zxiiro]

I'm a big fan of code reuse and not duplicating code. Considering configtool, talkeditor, recordingui, and reporteditor all base themselves on FreeseerApp is there no way to have them all use the same code to set the language rather than having the same 3 lines of code copied to all of the frontends?

[Cryspia]

That is what I thought! However, the import of self.config is after initializing FreeseerApp in those tools and editors. Which means I cannot make sure whether the user have a configuration file or not when FreeseerApp is initializing.

If it can be guaranteed that moving the config loading into FreeseerApp in the front of the initialization will not have inference on the program. I am glad to do that.

[zxiiro]

I say give it a try and see if anything breaks.

Edit: you can even do this test in a separate branch in case it does break and you have a path to go back to.

[mtomwing]

I'll also mention again that you don't need a dummy language. You can make your auto detected language be the default value in freeseer.settings.

[Cryspia]

The tricky thing is that if I do not use the dummy language, I will not be aware of whether the default language in the config is the initial value (the user does not have a config file yet) or the user chose that language by force (then I should not change that). Now if it is a dummy language. I know that the user does not have a config file.

Or should I add another method in the config class to return a bool of whether there is a config file?

[mtomwing]

You are over thinking this. The point of a default config value is that it will be used when there is no config file.

[mtomwing]

If this weren't the case, then your dummy language wouldn't even work.

[Cryspia]

Actually this function will only work when the user open Freeseer for the first time. At that time, there is definitely no config file. So the dummy language will work and the language will be auto detected.

Once there is a config file, it will be either that the language has already been auto chosen in the previous running or the user forced to change the default language in the ConfigTool.

Since the user's choice always has higher priority according to #601 , The auto language function will not run and the dummy language will also be not used.

[mtomwing]

What are your thoughts on this?

class FreeseerSettings(Config):
    ...
    default_language = options.StringOption(str(QLocale.system().name()))

[Cryspia]

@mtomwing That is a better idea than the dummy language. Thanks mtomwing! As long as the config loading can be done in FreeseerApp, I will change to that line.

[mtomwing]

Yeah, you can definitely refactor the config stuff to be in FreeseerApp.

[Cryspia]

@mtomwing I have passed the config argument to the FreeseeApp.init so that there is no more dummy setting. And the duplicated code in the frontends are also removed.

[Cryspia]

@mtomwing This function is checking whether the system language matches any of the language we offered. And the whole process is done in a loop of loading the languages in the menu. There might be duplication in code if I move it becauseI need to create a loop in freeseer.settings to check the language menu items.

[Cryspia]

This version has some problem, the QDir in settings.py does not read the target language files. i upload this version cause I cannot solve it, so i need someone help me.

[mtomwing]

Single quotes please.

[Cryspia]

@mtomwing Where do I need to change to Single quotes? I cannot find the review on the code.

[mtomwing]

Looks like you might have already fixed that during some other changes.

Some comments on your commit message:

• The "Fixes 476" in the body is redundant as it's already in your summary.
• You can simplify the body by talking about your changes at a higher level (e.g. When Freeseer is started for the first time, it will use the system language as the default language. If we do not have a translation in this language, en_US will be used.)
• The summary line doesn't describe what the commit introduces as a whole.

[Cryspia]

@mtomwing I am not very sure about how to rewrite the summary line. Does that look OK to you?

[Cryspia]

@mtomwing Comment changed to "system"

[dideler]

I think it would be useful to have tests for detect_system_language() since we often discover translation issues. You also get experience writing tests before the term ends.

[Cryspia]

@dideler You mean unit test? OK. I will work on that.

[dideler]

@Promm yes. You'll want to stub QLocale.system().name() so you can control what it returns and test both cases (when the language file exists and doesn't exist).

[Cryspia]

@dideler Can you give me some information about where to put the test and what is the style of the test?

[zxiiro]

@Promm Take a look at the src/freeseer/tests directory. You should be able to deduce how everything's organized out pretty quick.

[dideler]

@Promm this will also be useful: http://freeseer.readthedocs.org/en/latest/contribute/developers/testing.html

[Cryspia]

@dideler @zxiiro The unit test has been finished. I change the code in settings.py for that and I would like to discuss it with you.

• I replace the "QLocale.system().name()" by "QLocale.name()" because there seems to be no way to stub "QLocale.system()". Though I still think "QLocale.system()" is better under this situation, for the convenience of testing, I have to make this change.
• The "ja" problem, With I have discussed with zxiiro on IRC has come out. Using "QLocale.name()" will get "ja_JA" instead of "ja". So I have to add some special cases for that because there is no method in QLocale that can return only the language space.

[zxiiro]

What do you mean there's no way to "stub". Do you mean a way to fake the input?

Typically that's done through what's called mocking frameworks. Maybe you can monkey patch or something QLocale.system() to return whatever value you want for testing.

As for QLocale.name() returning ja_JA, what about the QLocale methods for QLocale.language() and QLocale.country()? Do they not do what you want or am I misunderstanding your question?

[dideler]

@Promm see http://pytest.org/latest/monkeypatch.html

[Cryspia]

@zxiiro For QLocale.language() and QLocale.country(), they both return an int. If I use QLocale.languageToString(QLocale.language()), I will get "Japanese" instead of "ja"

[Cryspia]

@dideler Thanks for telling me such powerful module.

[Cryspia]

@dideler Actually I met a problem with monkeypatch. I am trying to stub QLocale.system() as the code in this PR, but When run unit test, I get this failure:

____________ TestDetectSystemLanguage.test_detect_system_language _____________

self = <freeseer.tests.test_settings.TestDetectSystemLanguage testMethod=test_detect_system_language>

    def test_detect_system_language(self):
        '''Tests the detect_system_language methods

            Uses both positive test and negative test
            '''
        language = None
        setting_test_monkeypatch = monkeypatch()
        # Positive test
        languages = ["ar_EG", "de_DE", "en_US", "fr_CA",
                     "ja", "nl_NL", "sv_SE", "zh_CN", "zh_HK"]
        for language in languages:
            setting_test_monkeypatch.setattr(QLocale, "system",
                                             lambda x: QLocale(language))
>           self.assertEqual(detect_system_language(),
                            "tr_{}.qm".format(language))

freeseer\tests\test_settings.py:47: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    def detect_system_language():
        """Detect the system language"""

>       current_language = 'tr_{}'.format(QLocale.system().name())
E       TypeError: unbound method <lambda>() must be called with QLocale instance as first argument (got nothing instead)

freeseer\settings.py:44: TypeError

I have tried a lot of other code but still cannot solve it. Can you help me?

[dideler]

@Promm let's start by switching the test from using unittest to pytest and splitting up our tests into separate methods (it's good practice to not group your asserts under one test).

import pytest

from PyQt4.QtCore import QLocale

from freeseer.settings import detect_system_language

class TestSettings:
    def test_detect_system_language_translation_found(self, monkeypatch):
        """Tests detect_system_language() returns the appropriate translation filename for the system language."""
        pass

    def test_detect_system_language_translation_not_found(self, monkeypatch):
        """Tests detect_system_language() returns default if no translation found for system language."""
        pass

Note the monkeypatch parameters. I referenced the docs at http://pytest.org for help.

Let me know when you're this far and we'll move onto the actual monkeypatching.

[Cryspia]

@dideler The test file has been changed using pytest.

[dideler]

So we want QLocale.system().name() to return predetermined values during testing to make our lives easier. How do we do that? First let's figure out what exactly that statement is doing.

QLocale.system().name() has two method calls chained together, which means that name() is being called on whatever system() returns.

According to the QLocale docs, system() returns a QLocale object.

So now we know we're calling name() on a QLocale object. That means we have to monkeypatch QLocale so that when it receives a message for name, it returns whatever value we tell it to.

monkeypatch.setattr(QLocale, "name", lambda x: locale)

That's it. Here's a full test.

def test_detect_system_language_translation_found(self, monkeypatch):
    """Tests detect_system_language() returns the appropriate translation filename for the system language."""
    locales = ("ar_EG", "de_DE", "en_US", "fr_CA", "ja", "nl_NL", "sv_SE", "zh_CN", "zh_HK")
        for locale in locales:
            monkeypatch.setattr(QLocale, "name", lambda x: locale)
            assert detect_system_language() == "tr_{}.qm".format(locale)

[Cryspia]

@dideler Oh I understand. I seems to have misunderstood the principle of monkeypatch (I was thinking that it can only trace the code which call the classes and the methods with exactly the same order). It seems to be more powerful than I thought. Thanks a lot for the help.

[Cryspia]

I found that there are some unit tests fail under Windows. Under Windows I got:

======== 12 failed, 222 passed, 1 skipped, 1 xfailed in 15.64 seconds =========

Is that something that needs fix? If yes, I may open an issue about that.

[dideler]

@Promm can you show us which 12 tests fail on Windows? Opening an issue is probably best.

[Cryspia]

@dideler A new issue has been created.

647

[Cryspia]

@dideler The Japanese problem has been fixed

[dideler]

Merged as 19c85be57687a11aa70d55469bf57766e7b26d79, thanks for the contribution.