New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for mappings in gettext calls #143

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@mattias-lidman

mattias-lidman commented Apr 28, 2017

Trying to use variables with gettext calls currently blows up with a TypeError. This change adds the ability to use mappings as described in http://docs.pylonsproject.org/projects/pyramid/en/latest/narr/i18n.html#using-the-translationstring-class.

@bjmc-globus

This comment has been minimized.

Show comment
Hide comment
@bjmc-globus

bjmc-globus Jul 26, 2018

@mmerickel do you have time to take a look at this PR? Thanks!

bjmc-globus commented Jul 26, 2018

@mmerickel do you have time to take a look at this PR? Thanks!

@mattias-lidman

This comment has been minimized.

Show comment
Hide comment
@mattias-lidman

mattias-lidman Jul 26, 2018

Looks like the Travis failure is a config issue:

We couldn't find the repository
Pylons/pyramid_jinja2

mattias-lidman commented Jul 26, 2018

Looks like the Travis failure is a config issue:

We couldn't find the repository
Pylons/pyramid_jinja2

@stevepiercy

This comment has been minimized.

Show comment
Hide comment
@stevepiercy

stevepiercy Jul 26, 2018

Member

Nope, it's this:
https://travis-ci.org/Pylons/pyramid_jinja2/jobs/408715774#L475-L483

I would guess that we need to update .travis.yml to use a matrix, like we use for other Pylons Projects, like Pyramid's .travis.yml.

I can do that, but before I do, should I drop support for Python 3.3? I'd like to do it all in one swoop.

Member

stevepiercy commented Jul 26, 2018

Nope, it's this:
https://travis-ci.org/Pylons/pyramid_jinja2/jobs/408715774#L475-L483

I would guess that we need to update .travis.yml to use a matrix, like we use for other Pylons Projects, like Pyramid's .travis.yml.

I can do that, but before I do, should I drop support for Python 3.3? I'd like to do it all in one swoop.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Jul 26, 2018

Member

@mattias-lidman I think the request was to accept mapping, not **mapping.

Member

mmerickel commented Jul 26, 2018

@mattias-lidman I think the request was to accept mapping, not **mapping.

@stevepiercy stevepiercy referenced this pull request Jul 28, 2018

Merged

Bump python versions #146

@stevepiercy

This comment has been minimized.

Show comment
Hide comment
@stevepiercy

stevepiercy Jul 28, 2018

Member

I've updated necessary bits to get Travis not to fail in #146, but the build is still failing. Perhaps another commit per #143 (comment) will result in success?

Member

stevepiercy commented Jul 28, 2018

I've updated necessary bits to get Travis not to fail in #146, but the build is still failing. Perhaps another commit per #143 (comment) will result in success?

@@ -18,10 +18,11 @@ def localizer(self):
except AttributeError: # pragma: nocover (pyramid < 1.5)
return i18n.get_localizer(request)
def gettext(self, message):
def gettext(self, message, **mapping):

This comment has been minimized.

@mmerickel

mmerickel Jul 28, 2018

Member

Please change this to mapping to avoid future compatibility issues when we want to add other arguments like domain/context.

Also, shouldn't this be added to ngettext as well?

@mmerickel

mmerickel Jul 28, 2018

Member

Please change this to mapping to avoid future compatibility issues when we want to add other arguments like domain/context.

Also, shouldn't this be added to ngettext as well?

This comment has been minimized.

@stevepiercy

stevepiercy Aug 18, 2018

Member

@mattias-lidman would you please update your PR as requested?

@stevepiercy

stevepiercy Aug 18, 2018

Member

@mattias-lidman would you please update your PR as requested?

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