TurboGears / tg2

Python web framework with full-stack layer implemented on top of a microframework core with support for SQL DBMS, MongoDB and Pluggable Applications
http://www.turbogears.org/
Other
806 stars 78 forks source link

Register gettext callables for the jinja2 renderer #36

Closed Turbo87 closed 11 years ago

Turbo87 commented 11 years ago

As a workaround currently the following code seems to work:

def install_gettext_callables(app):
    from tg.i18n import ugettext, ungettext
    jinja2_env = app_globals.config['pylons.app_globals'].jinja2_env
    jinja2_env.install_gettext_callables(ugettext, ungettext)
    return app

base_config.register_hook('after_config', install_gettext_callables)

This patch has not been tested yet due to missing testing environment though. I'd be glad if someone could help me with that...

Turbo87 commented 11 years ago

I've just quickstarted a new project including jinja support and it seems that this patch is acutally not needed. It seems that the problem in my other project is somewhere else. I'm currently converting a project from genshi to jinja2, but as you can see it is not quite as easy as I hoped...

Turbo87 commented 11 years ago

it seems that the problem was originally related to the "with context" thing of Jinja2 and after I added it to the imports I was doing the _() worked. now I'm experiencing the next problem though which is that {% trans %} doesnt seem to be working. when I add my workaround to the app_cfg.py it seems to work alright.

the resulting error is:

UndefinedError: 'gettext' is undefined

this can be reproduced with tg2.2.2:

  1. create a new project with paster quickstart and use the jinja2 template engine
  2. add 'jinja2.ext.i18n' to base_config.jinja_extensions in app_cfg.py
  3. add a {% trans %} tag somewhere
  4. call the page and see it crash because gettext is undefined
amol- commented 11 years ago

@clsdaniel as the main jinja support contributor I would like to have your feedback before merging in this. Looks like that calling install_gettext_callables after enabling i18n extension should do the trick, but I would like a jinja user review this.

Turbo87 commented 11 years ago

I'm a bit worried that the travis tests are not passing... is that expected? as I mentioned in the initial message I have not tested this patch, only my workaround for tg2.2.2.

clsdaniel commented 11 years ago

I'm checking this up, been busy and would have liked to help more on this, let me check it.

clsdaniel commented 11 years ago

I see this patch is against master, wouldn't it be better to get it in development for the next minor release?

I tried on my machine with development branch, I see that adding jinja2.ext.i18n is necessary otherwise it will not let you install the gettext callables, so adding something like this after checking for jinja_extensions should be in order:

if not "jinja2.ext.i18n" in self.jinja_extensions:
    self.jinja_extensions.append("jinja2.ext.i18n")

I think we should have Jinja i18n extensions fully configured out of the box (as is with Gensh), perhaps with tests. Finally I'll check if translations are really being done.

Turbo87 commented 11 years ago

I'm happy to refactor the patch and target it at another branch if that is requested. Let me know how you decide.

clsdaniel commented 11 years ago

I would like @amol- opinion as this patch will not fly under development branch as there has been work to remove direct dependencies on pylons, gettext calls for example are fetched from tg.i18n instead of pylons module. I'm happy to review and help get the patch merged.

amol- commented 11 years ago

Unless major bugs are discovered there won't be any new 2.2.X release, so the patch should probably go to the development branch for inclusion in 2.3.0.

Porting it should be fairly simple, just keep in mind that 2.3 branch is currently 100% code covered by test units and works on py2.6->py3.3 so it will be required to write tests to keep full coverage and check that it works on py3.

Turbo87 commented 11 years ago

opened pull request #37 to the development branch. let's continue the discussion there.