simplify locales #818

ccanepa · 2013-10-11T15:16:04Z

This change revamps the locale handling in Nikola.

The goals

compatibility with windows
consolidate some disperse and partially duplicated code
give the user the option about which exact locale use with each languagen
make configurable the languages and locales used in testing

The change in a nutshell

Sanitized locales are calculated when conf.py is first loaded; after that the code deals only with sanitized locales.
Sanitized locales and services related to locales live in nikola.utils.LocaleBorg
LocaleBorg is also the authority for .current_lang
Other pieces of code that were variants of 'current_lang ?' eliminated or defer to LocaleBorg().current_lang
Languages and locales to use in testing can be specified with the environment variables NIKOLA_LOCALE_DEFAULT and NIKOLA_LOCALE_OTHER

Backward compatibility

At the level of conf.py , full compatibility in installations where locale support was working
At spec level: theres what I think a minor and reasonable restriction about what locales you can set: you can choose between the locales declared (explicitly or not) in conf.py
testing uses 'en': 'en_US' by default

api compatibility

deleted nikola.nikola.s_l (the templates get the functionality trough LocaleBorg.set_locale, transparently)
deleted get_month_name (moved to LocaleBorg.get_month_name, uses in nikola code adjusted
Post.current_lang deleted, uses replaced with direct invocations of LocaleBorg().current_lang

For strict compatibility I should have deprecated and retained those deleted, serving people who subclassed some parts of nikola.
I guessed there are no people using directly those functions but if desired, I can undelete those.

new feature
User can specify the exact locale to use with each language in conf.py
There are three new optional settings in conf.py related to locales

LOCALES: dict of lang: locale key-value pairs. His keys must be keys of TRANSLATIONS. Some or all keys can be omitted.

LOCALE_FALLBACK : locale to use when an explicit locale is invalid (if not set, two hardcoded locales are tried, "en" and "C", the later should be available in any host)

LOCALE_DEFAULT: guides the handling of implicit locales

An explicit locale for a language can be specified in LOCALE[language].
Explicit but invalid locales are replaced with the sanitized locale_fallback
Languages with no explicit locale are set to

the sanitized locale_default if it was explicitly set
sanitized guesses compatible with v 6.0.4 if locale_default was None (or not present in conf.py)

Misc

all tests (except doctests) green in windows

Kwpolska · 2013-10-11T15:28:37Z

nikola/utils.py

+            locale_n = cls.locales[lang]
+            cls.__shared_state['current_lang'] = lang
+            locale.setlocale(locale.LC_ALL, locale_n)
+        return ''


None would be a better idea.

(None is also the default return value btw)

No, it is used in templates, ralsina commented in some mail it needs to be
a string

On Fri, Oct 11, 2013 at 12:28 PM, Chris “Kwpolska” Warrick <
notifications@github.com> wrote:

In nikola/utils.py:

self.__dict__ = self.__shared_state

@classmethod

def set_locale(cls, lang):

"""Sets the locale for language lang, returns ''

in linux the locale encoding is set to utf8,

in windows that cannot be guaranted.

In either case, the locale encoding is available in cls.encodings[lang]

"""

# intentional non try-except: templates must ask locales with a lang,

# let the code explode here and not hide the point of failure

if lang != cls.__shared_state['current_lang']:

locale_n = cls.locales[lang]

cls.__shared_state['current_lang'] = lang

locale.setlocale(locale.LC_ALL, locale_n)

return ''

None would be a better idea.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/818/files#r6918164
.

This one is called from templates, if it returns None you end up with 'None' in the middle of the page :-/

Kwpolska · 2013-10-11T15:46:01Z

~~Twelve~~ Eighteen of the tests on Travis and nikola help managed to crash. https://travis-ci.org/getnikola/nikola/jobs/12423224#L811

ccanepa · 2013-10-11T15:50:01Z

out for lunch, will look when back

On Fri, Oct 11, 2013 at 12:46 PM, Chris “Kwpolska” Warrick <
notifications@github.com> wrote:

Twelve of the tests on Travis and nikola help managed to crash.
https://travis-ci.org/getnikola/nikola/jobs/12423224#L811

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/818#issuecomment-26147680
.

ccanepa · 2013-10-11T16:59:44Z

Silly me, I forgot to load locales in Travis. Updated, lets see how it comes.
Also expanded the import abbreviations.

Kwpolska · 2013-10-11T17:39:39Z

I’d like to point you at https://travis-ci.org/getnikola/nikola/jobs/12428498#L1085 — is this correct?

ccanepa · 2013-10-11T17:57:33Z

kw: yes, it is expected output when test_locale stress the sanitization subsystem with a purposely invalid locale

ccanepa · 2013-10-11T18:56:03Z

Lets hear Travis opinion.
I'm a bit nervous about the interaction of the magic nikola import and the import nikola.utils ; related or not I needed to add an # NOQA to the magic nikola import.

Also: I situated the provision of samples to initialize LocaleBorg in testing in the same LocaleBorg; that could be moved to tests\base or somewhere if desired.

ccanepa · 2013-10-11T19:05:01Z

Green and fresh as a lettuce !

I will hear request for changes, but be patient, I need to do work related duties and probably cannot retake until tomorrow (Saturday) evening.

ralsina · 2013-10-11T20:14:23Z

If you are going to use classmethods in localeBorg, then it can be just a regular class, and the init can take place outside of it, in the Nikola class, using the class just as a namespace.

ccanepa · 2013-10-12T07:11:41Z

About classmethods and other things LocaleBorg

Now that all the pieces are in place a rethinking of the classification in class / instance methods is in order.

I think

initialize
reset
initialize_for_testing

are strong candidates for class methods:
first and last are constructors (for shared state)
reset is a poor man constructor for the initial, uninitialized state that don't needs to call _reload(utils), it feels like the right thing to call at tearDown

The first two are also strong candidates to live in LocaleBorg, because they need to know the internal details of LocaleBorg.
And, in particular initialize, it should stay in LocaleBorg, not in Nikola class: a small unit like LocaleBorg shouldn't need to load a big class as Nikola only to initialize in testing, nor should need a semi-duplicated code in testing to initialize.

The third (initialize_for_testing) simply uses the other two, so not strong case to live in LocaleBorg.
Weak points in favor of initialize_for_testing in LocaleBorg

short and clear in code and naming, so it not obscures the understanding of the borg functionality
provides concrete examples of what input expects initialize
testing convenience
it is there now :-)

The other methods, the ones providing services, I think they want to be instance methods.

if you need an instance to call a service, then you called init, which checks that state has been properly initialized
that spares us to check in each service call the initialization state.

Theres not a strong case to ensure initialization in this way, we surely are trading a bit of cpu an memory overhead for sparing a bit of boilerplate code.

So, meh. I have a small preference to turning services in instance methods, but thats all.

Concrete questions to you, people:

objections to make services as instance methods ?
should I move initialize_for_testing to tests/base.py ? leave in LocaleBorg ?
whatever the details of this decisions, I want to code them and maybe do a squash of all the commits in this PR (so github shows diffs of the overal change to further discuss). Any advice in contrary ?

Kwpolska · 2013-10-12T07:45:58Z

Do not squash the commits. GitHub will show you a nice overall diff without this: https://github.com/getnikola/nikola/pull/818/files

Author: Claudio Canepa <ccanepacc@gmail.com>

…in imports

…KOLA_LOCALE_DEFAULT and NIKOLA_LOCALE_OTHER

ccanepa · 2013-10-16T16:13:45Z

^^ PR updated.

Rebasing branch on master shows (in the prev post) as lot of new commits but the only new are the last three.
I think this PR is ready to land.
Comments ?

ralsina · 2013-10-16T17:08:59Z

I'll try to do a real review of it tonight.

Kwpolska · 2013-10-16T18:39:58Z

nikola/conf.py.in

+# In this file we express locales in the string form that
+# python's locales will accept in your OS, by example
+# "en_US.utf8" in unix-like OS, "English_United States" in Windows. 
+# LOCALES = dict mapping languaje --> explicit locale for the languajes 


Kwpolska · 2013-10-16T18:49:06Z

nikola/nikola.py

+
+_windows_locale_guesses = {
+    # some languages may need that the appropiate Microsoft's Language Pack
+    # be instaled; the 'str' bit will be added in the guess function


I’d love to see some documentation about this “need to have language pack”. As far as I know, Windows ships all locales (as in date/time/currency/…) and installs them all by default without an easy way to uninstall them. (this differs from UI languages, see below).

UI Languages:

Windows 2000 (Professional/Server) and Windows XP Professional had somehow-working language packs that were hard to obtain and use

Windows Vista/7 Ultimate/Enterprise had sane language packs everyone could download from Windows Update

Windows 8/RT has even saner language packs that are downloaded through a separate Control Panel applet and are available for everyone (not just rich people who bothered to buy Ultimate or even richer companies with Enterprise)

I phrased as 'may need'.
I don't really expect that my spanish windows xp / 7 supports out of the box anything chinese.

I’m going to test it on my almost-vanilla XP/7 VMs and share the results.

And here we have the answer:

Windows XP requires East Asian language support that can be easily installed in the locale control panel (Regional & Language Settings). That requires a Windows XP CD, something I do not own (using a VM from http://modern.ie/)

Windows 7 has built-in support. Proof:

(for those wondering, this copy of Windows is unactivated and comes from a genuine install CD I own. It is almost-vanilla with minimum additional software.)

PS. the same is happening in the Polish locales of both (XP — tried myself, 7 — a friend told me that he had Japanese locale because he needed it for something ootb on a Polish Windows; I never owned a copy of Windows 7 in Polish and never will)

ccanepa · 2013-10-16T23:11:05Z

out of battery, phone line died monday night so letting here.
will check latter for more feedback

ralsina · 2013-10-17T15:09:10Z

I like it. +1 Great work @ccanepa I know this kind of fixes are hard and a lot of work :-)

simplify locales

ccanepa · 2013-10-18T04:52:31Z

Thanks you both for the reviews, git advice and general patience.

Kwpolska reviewed Oct 11, 2013
View reviewed changes

ccanepa added 8 commits October 16, 2013 12:38

simplify locales

67c5ff3

Author: Claudio Canepa <ccanepacc@gmail.com>

updatet .travis.yml to support two locales - eliminated abreviations …

01d27cd

…in imports

compat 2.x in locale linux codepath

241c51f

test_rss_feeds initializes LocaleBorg in setUp

a0a630e

LocaleBorg services as instance methods to ensure initialization

a035cb5

reshaping paranoid damage control forcalendar in windows

4c486a7

allow configure which locales in testing with environmet variables NI…

728c7ee

…KOLA_LOCALE_DEFAULT and NIKOLA_LOCALE_OTHER

update changelog

2c34887

Kwpolska reviewed Oct 16, 2013
View reviewed changes

addresed kw review

44abdc6

fixed remaining _linux -> _posix

b5ef20a

Kwpolska mentioned this pull request Oct 17, 2013

Pb when creating monthly archive in french language #842

Closed

ralsina added a commit that referenced this pull request Oct 17, 2013

Merge pull request #818 from ccanepa/better-locale

968d7f3

simplify locales

ralsina merged commit 968d7f3 into getnikola:master Oct 17, 2013

ccanepa mentioned this pull request Oct 18, 2013

Simpler and more robust locale handling #771

Closed

ccanepa deleted the better-locale branch October 18, 2013 04:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

simplify locales #818

simplify locales #818

ccanepa commented Oct 11, 2013

Kwpolska Oct 11, 2013

ccanepa Oct 11, 2013

ralsina Oct 11, 2013

Kwpolska commented Oct 11, 2013

ccanepa commented Oct 11, 2013

ccanepa commented Oct 11, 2013

Kwpolska commented Oct 11, 2013

ccanepa commented Oct 11, 2013

ccanepa commented Oct 11, 2013

ccanepa commented Oct 11, 2013

ralsina commented Oct 11, 2013

ccanepa commented Oct 12, 2013

Kwpolska commented Oct 12, 2013

ccanepa commented Oct 16, 2013

ralsina commented Oct 16, 2013

Kwpolska Oct 16, 2013

ccanepa Oct 16, 2013

Kwpolska Oct 16, 2013

ccanepa Oct 16, 2013

Kwpolska Oct 17, 2013

Kwpolska Oct 17, 2013

ccanepa commented Oct 16, 2013

ralsina commented Oct 17, 2013

ccanepa commented Oct 18, 2013

simplify locales #818

simplify locales #818

Conversation

ccanepa commented Oct 11, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kwpolska commented Oct 11, 2013

ccanepa commented Oct 11, 2013

ccanepa commented Oct 11, 2013

Kwpolska commented Oct 11, 2013

ccanepa commented Oct 11, 2013

ccanepa commented Oct 11, 2013

ccanepa commented Oct 11, 2013

ralsina commented Oct 11, 2013

ccanepa commented Oct 12, 2013

Kwpolska commented Oct 12, 2013

ccanepa commented Oct 16, 2013

ralsina commented Oct 16, 2013

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

ccanepa commented Oct 16, 2013

ralsina commented Oct 17, 2013

ccanepa commented Oct 18, 2013