i18n change #14

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
@jeffmad

jeffmad commented Jan 16, 2011

This change corresponds to my other change for i18n. I appreciate any feedback.

jeffmad added some commits Jan 16, 2011

jeffmad
in order to get i18n to work with pyramid / jinja2, the translations …
…object must be passed to the jinja2 environment. added code to check if i18n extension is enabled, then if it is enabled, we pull the localizer from the request and install the translations object into the jinja2 environment.
@mcdonc

This comment has been minimized.

Show comment
Hide comment
@mcdonc

mcdonc Jan 21, 2011

Member

Hi Jeff, sorry I haven't gotten around to reviewing this yet.. trying to get Pyramid core out the door, and once I do, I'll come back to this.

Member

mcdonc commented Jan 21, 2011

Hi Jeff, sorry I haven't gotten around to reviewing this yet.. trying to get Pyramid core out the door, and once I do, I'll come back to this.

@mcdonc

This comment has been minimized.

Show comment
Hide comment
@mcdonc

mcdonc Jan 27, 2011

Member

Moved from the Pyramid bugtracker at https://github.com/Pylons/pyramid/issues#issue/102 (by jeffmad) to here, as this is not a Pyramid core issue:

Hi,
Please consider this change coupled with another forthcoming change i have for pyramid_jinja2 to get i18n working with jinja2. As it stands the way that get_localizer works, a default Translation object is created and all other translations are added to this. The consequences of this is that you must pass in the domain to get a translation out, which jinja2 does not do. My proposed fix will postpones the creation of the Translations object until after the translation dirs are searched. This allows one to do lookups without passing in the domain for at least one of the translation objects.

Then in pyramid_jinja2/init.py I install the localizer.translations into the Jinja2 enviornment.

I tried to make the docs as close in tone as i could. I appreciate any feedback that you have.
--jeff

Member

mcdonc commented Jan 27, 2011

Moved from the Pyramid bugtracker at https://github.com/Pylons/pyramid/issues#issue/102 (by jeffmad) to here, as this is not a Pyramid core issue:

Hi,
Please consider this change coupled with another forthcoming change i have for pyramid_jinja2 to get i18n working with jinja2. As it stands the way that get_localizer works, a default Translation object is created and all other translations are added to this. The consequences of this is that you must pass in the domain to get a translation out, which jinja2 does not do. My proposed fix will postpones the creation of the Translations object until after the translation dirs are searched. This allows one to do lookups without passing in the domain for at least one of the translation objects.

Then in pyramid_jinja2/init.py I install the localizer.translations into the Jinja2 enviornment.

I tried to make the docs as close in tone as i could. I appreciate any feedback that you have.
--jeff

@mcdonc

This comment has been minimized.

Show comment
Hide comment
@mcdonc

mcdonc Jan 27, 2011

Member

Note that there is a pull request attached to Pylons/pyramid#102 ( https://github.com/jeffmad/pyramid/commit/e718d34a5372be2a9e791dfaf601217ae581496e ) that includes Pyramid core changes. These may need to be incorporated.

Member

mcdonc commented Jan 27, 2011

Note that there is a pull request attached to Pylons/pyramid#102 ( https://github.com/jeffmad/pyramid/commit/e718d34a5372be2a9e791dfaf601217ae581496e ) that includes Pyramid core changes. These may need to be incorporated.

@rockyburt

This comment has been minimized.

Show comment
Hide comment
@rockyburt

rockyburt Feb 13, 2011

Member

Once (if) the pyramid core changes pull request is accepted, I would definitely consider including the changes here into pyramid_jinja2. But... as it stands, this set of commits lack test coverage. So my suggestion would be to be to create a new pull request by first updating changes to go along with current pyramid_jinja2 master and then adding tests.

Member

rockyburt commented Feb 13, 2011

Once (if) the pyramid core changes pull request is accepted, I would definitely consider including the changes here into pyramid_jinja2. But... as it stands, this set of commits lack test coverage. So my suggestion would be to be to create a new pull request by first updating changes to go along with current pyramid_jinja2 master and then adding tests.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment