Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(18)

Issue 3340041: #2504: Add gettext.pgettext and friends

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 7 months ago by merwok
Modified:
4 years, 3 months ago
Reviewers:
andrew.kuchling
Visibility:
Public.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+564 lines, -51 lines) Patch
M Doc/library/gettext.rst View 13 chunks +155 lines, -1 line 0 comments Download
M Lib/gettext.py View 14 chunks +145 lines, -9 lines 1 comment Download
M Lib/test/test_gettext.py View 12 chunks +231 lines, -27 lines 1 comment Download
M Misc/ACKS View 1 chunk +1 line, -0 lines 0 comments Download
M Misc/NEWS View 1 chunk +3 lines, -0 lines 0 comments Download
M Tools/i18n/msgfmt.py View 9 chunks +29 lines, -14 lines 0 comments Download

Messages

Total messages: 1
andrew.kuchling
4 years, 3 months ago (2015-04-14 15:24:19 UTC) #1
The patch looks generally fine to me.

https://codereview.appspot.com/3340041/diff/1/Lib/gettext.py
File Lib/gettext.py (right):

https://codereview.appspot.com/3340041/diff/1/Lib/gettext.py#newcode383
Lib/gettext.py:383: except KeyError:
To minimize the risk of hiding unrelated KeyErrors, should the 'if
self._output_charset ... return tmsg' lines be moved outside of the 'try' block
and put into an 'else' clause?

https://codereview.appspot.com/3340041/diff/1/Lib/test/test_gettext.py
File Lib/test/test_gettext.py (right):

https://codereview.appspot.com/3340041/diff/1/Lib/test/test_gettext.py#newcod...
Lib/test/test_gettext.py:102: def test_some_translations_with_context(self):
The test suite modifications don't seem to exercise all of the variants
introduced by the patch, e.g. lpgettext, npgettext, lnpgettext, ldnpgettext.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b