Auto-detect language #609

Cryspia · 2014-10-04T08:54:36Z

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 · 2014-10-04T09:34:20Z

It is so hard to pass the build :(

mtomwing · 2014-10-04T18:25:14Z

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

Cryspia · 2014-10-05T00:01:11Z

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 · 2014-10-05T01:20:43Z

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 · 2014-10-05T04:18:53Z

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 · 2014-10-05T19:05:24Z

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 · 2014-10-05T19:21:58Z

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 · 2014-10-05T23:10:56Z

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 · 2014-10-05T23:20:22Z

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 · 2014-10-05T23:23:54Z

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

Cryspia · 2014-10-06T01:03:16Z

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 · 2014-10-06T07:33:28Z

What are your thoughts on this?

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

Cryspia · 2014-10-07T07:13:56Z

@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 · 2014-10-07T07:45:47Z

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

Cryspia · 2014-10-13T03:52:03Z

@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.

mtomwing · 2014-10-13T04:02:52Z

src/freeseer/settings.py

@@ -26,6 +26,7 @@

 from freeseer.framework.config.core import Config
 from freeseer.framework.config.profile import ProfileManager
+from PyQt4.QtCore import QLocale


Imports are ordered like so:

# modules in stdlib import os # third party modules import PyQt # freeseer modules import freeseer

Cryspia · 2014-10-13T05:17:01Z

@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 · 2014-10-13T08:24:16Z

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.

zxiiro · 2014-10-14T00:50:38Z

src/freeseer/settings.py

+            if current_language[3:5] == str(language)[3:5]:
+                language_detected = True
+                #Additional case for Simplified Chinese & Traditional Chinese.
+                #Traditional Chinese has higher priority


I don't understand why we need a special case for Simplified and Traditional. In addition I also don't understand why Traditional has higher priority? can you explain this?

Also when doing comments it's good practice to leave a space between the pound (#) and your text.

Since I does not consider the Location of the system when deciding which language to use (so fr_FR and fr_CH will be changed to fr_CA), there will be conflict on the two Chinese language. If I do not do the 'if case', all the Chinese system will choose Traditional Chinese because it lists after the Simplified Chinese in the loop.

About the high priority, it is because that for all the locations that use Chinese, only mainland China uses Simplified Chinese. TW, HK and some South East Asia countries use Traditional Chinese. So I set Traditional Chinese a higher priority and do a special judgement for zh_CN (mainland China).

Sorry I think I still don't follow but I'll explain how I understand the auto detection works and you can tell me what makes sense.

As I understand it we're supposed to be auto-detecting the user's OS language that the user set due to line 76 when we call QLocale.system().name(). So if your system Locale is en_US then you get US English, if your locale is en_CA then you should get Canadian English.

When I installed my OS I picked my default language as part of the install process so in my case I'm on en_CA so my locale should always report en_CA for my user (unless I changed it in my user profile). So Freeseer should be getting my correct Locale when it calls QLocale.system().name().

So I would imagine in the case of the Chinese writing systems, I would imagine when the user installed their OS (or by user preferences post install) they would have selected to install Simplified or Traditional Chinese thus the correct zh_CN or zh_HK would already be reported to Freeseer via the Locale variable.

Are you saying we cannot trust the user's OS level Locale setting?

It's probably sufficient just to check if the translation file exists (os.path.isfile) rather than use QDir.

mtomwing · 2014-10-19T18:11:24Z

src/freeseer/settings.py

+    current_language = 'tr_{}.qm'.format(str(QLocale.system().name()))
+    language_path = os.path.join(os.path.dirname(os.path.realpath(__file__)),
+                                 'frontend', 'qtcommon', 'languages', "{}.ts"
+                                 .format(current_language.rstrip('.qm')))


It doesn't seem like you need the .qm suffix until you return, so it would be better to add it there.

dideler · 2014-11-07T00:23:25Z

src/freeseer/tests/test_settings.py

+            assert detect_system_language() == "tr_{}.qm".format(language)
+
+    def test_detect_system_language_translation_not_found(self, monkeypatch):
+        '''Tests the detect_system_language methods using negative way.


Use the docstring I provided in the example, it's more descriptive.

dideler · 2014-11-07T00:42:12Z

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 · 2014-11-07T01:44:36Z

@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 · 2014-11-07T02:12:08Z

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 · 2014-11-07T08:30:25Z

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

dideler · 2014-11-07T08:33:16Z

src/freeseer/tests/test_settings.py

+
+    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",


This can fit on one line.

Cryspia · 2014-11-07T21:11:15Z

@dideler
A new issue has been created.
#647

dideler · 2014-11-11T18:38:50Z

src/freeseer/settings.py

+    translation = 'tr_{}'.format(QLocale.system().name())
+    translation_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'frontend', 'qtcommon', 'languages')
+    translation_country_language = os.path.join(translation_path, '{}.ts'.format(translation))
+    translation_language = os.path.join(translation_path, '{}.ts'.format(translation.split('-')[0]))


Shouldn't this be splitting on underscore _, not dash -? E.g. tr_en_US

I'm guessing the build on Travis is old since it didn't fail.

Edit: I'm tired and lost, what exactly are you trying to do here? What's the elif case for?

Edit 2: So it looks like locales on Windows use - while on Linux they use _. Can you confirm? Also, since ja_JP/ja-JP is a valid locale, is the if/elif logic still necessary?

- Freeseer will now scan the System language for the first time. If there is translation for the location, the interface language will be set to that language by default. Otherwise en_US will be used. - Unit test for this function is also added..

Cryspia · 2014-11-13T11:18:31Z

@dideler
The Japanese problem has been fixed

dideler · 2014-11-13T13:48:46Z

Merged as 19c85be, thanks for the contribution.

dideler added this to the 2014 Fall - UCOSP milestone Oct 7, 2014

Cryspia force-pushed the 476-auto-language-changing branch from 30cc117 to 9e09fbd Compare October 12, 2014 23:48

mtomwing reviewed Oct 13, 2014
View reviewed changes

zxiiro reviewed Oct 14, 2014
View reviewed changes

Cryspia force-pushed the 476-auto-language-changing branch from 4b4a271 to 4a137f4 Compare October 19, 2014 05:35

mtomwing reviewed Oct 19, 2014
View reviewed changes

dideler reviewed Nov 7, 2014
View reviewed changes

Cryspia force-pushed the 476-auto-language-changing branch 2 times, most recently from 0553785 to 6a34cc1 Compare November 7, 2014 02:07

Cryspia force-pushed the 476-auto-language-changing branch from 6a34cc1 to db93070 Compare November 7, 2014 05:21

dideler reviewed Nov 7, 2014
View reviewed changes

Cryspia force-pushed the 476-auto-language-changing branch from db93070 to 2fd1954 Compare November 7, 2014 21:00

dideler mentioned this pull request Nov 8, 2014

Fix #153 - Log / Error Notification System #635

Merged

9 tasks

dideler reviewed Nov 11, 2014
View reviewed changes

Cryspia force-pushed the 476-auto-language-changing branch from 2fd1954 to 7e96e53 Compare November 13, 2014 10:34

Cryspia force-pushed the 476-auto-language-changing branch from 7e96e53 to 67f5e40 Compare November 13, 2014 11:17

dideler changed the title ~~Fix #476 Auto language changing function finished.~~ Auto-detect language Nov 13, 2014

dideler closed this Nov 13, 2014

Cryspia deleted the 476-auto-language-changing branch November 14, 2014 00:52

dideler mentioned this pull request Dec 13, 2014

Suggestion: change the default language while the user select a language #600

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto-detect language #609

Auto-detect language #609

Cryspia commented Oct 4, 2014

Cryspia commented Oct 4, 2014

mtomwing commented Oct 4, 2014

Cryspia commented Oct 5, 2014

zxiiro commented Oct 5, 2014

Cryspia commented Oct 5, 2014

zxiiro commented Oct 5, 2014

mtomwing commented Oct 5, 2014

Cryspia commented Oct 5, 2014

mtomwing commented Oct 5, 2014

mtomwing commented Oct 5, 2014

Cryspia commented Oct 6, 2014

mtomwing commented Oct 6, 2014

Cryspia commented Oct 7, 2014

mtomwing commented Oct 7, 2014

Cryspia commented Oct 13, 2014

mtomwing Oct 13, 2014

Cryspia commented Oct 13, 2014

Cryspia commented Oct 13, 2014

zxiiro Oct 14, 2014

Cryspia Oct 14, 2014

zxiiro Oct 14, 2014

mtomwing Oct 14, 2014

mtomwing Oct 19, 2014

dideler Nov 7, 2014

dideler commented Nov 7, 2014

Cryspia commented Nov 7, 2014

Cryspia commented Nov 7, 2014

dideler commented Nov 7, 2014

dideler Nov 7, 2014

Cryspia commented Nov 7, 2014

dideler Nov 11, 2014

Cryspia commented Nov 13, 2014

dideler commented Nov 13, 2014

Auto-detect language #609

Auto-detect language #609

Conversation

Cryspia commented Oct 4, 2014

Cryspia commented Oct 4, 2014

mtomwing commented Oct 4, 2014

Cryspia commented Oct 5, 2014

zxiiro commented Oct 5, 2014

Cryspia commented Oct 5, 2014

zxiiro commented Oct 5, 2014

mtomwing commented Oct 5, 2014

Cryspia commented Oct 5, 2014

mtomwing commented Oct 5, 2014

mtomwing commented Oct 5, 2014

Cryspia commented Oct 6, 2014

mtomwing commented Oct 6, 2014

Cryspia commented Oct 7, 2014

mtomwing commented Oct 7, 2014

Cryspia commented Oct 13, 2014

Choose a reason for hiding this comment

Cryspia commented Oct 13, 2014

Cryspia commented Oct 13, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dideler commented Nov 7, 2014

Cryspia commented Nov 7, 2014

Cryspia commented Nov 7, 2014

dideler commented Nov 7, 2014

Choose a reason for hiding this comment

Cryspia commented Nov 7, 2014

Choose a reason for hiding this comment

Cryspia commented Nov 13, 2014

dideler commented Nov 13, 2014