You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee='https://github.com/merwok'closed_at=<Date2010-12-03.19:26:08.157>created_at=<Date2010-11-22.02:31:24.784>labels= ['type-bug', 'library']
title='Incorrect use of gettext in argparse'updated_at=<Date2010-12-03.19:26:08.156>user='https://github.com/merwok'
The patch makes sense. I wonder if i10n of programs using argparse should be mentioned in docs. Providing a sample translation file somewhere may not be a bad idea at least for testing.
bethard commented on Nov 24, 2010
bethardmannequin
Mannequin
More actions
Looks like a great fix.
And yes, anyone who knows anything about gettext, please feel free to add a test or ten. ;-) I just copied optparse when I put that stuff in, so I have no confidence in how it's done now.
A bit of rationale behind the patch. You probably know that the gettext/_ functions serves two roles: Identifying the strings to translate, and retrieving the translation from a catalog.
At strings extraction time, the string in the line
msg = _('unknown parser %r (choices: %s)' % tup)
will be found by xgettext (just checked), so the .pot and .po files will include it. So far, so good.
At runtime however, the gettext function will get "unknown parser 'parserr' (choices: ('some', 'tuple'))" as argument, which isn’t in the translation catalogs. Doing the string interpolation after the gettext call (my patch) avoids this problem.
I’ve started on a test but I’m not sure how to proceed, since I need to use or intercept gettext and compare the error messages of argparse with translated messages to prove that the gettext calls are bogus. There are various ways of using or intercepting gettext from tests, but I have to experiment a bit before I can propose the simplest patch that could possibly work.
Exceptionally, I’d like to commit the change as is, and defer the testing for some months, until I can get to bpo-10529.
Yes, I think it's okay to fix this without a test, given that it's a nontrivial amount of work to test gettext stuff. I'd rather have it working now, without tests, than wait until we know how to test stuff with gettext.
It's also a pretty minor, obvious fix. Of course, I assume you'll make sure this also merges correctly with your bpo-10528 patch.
From the other bug: “If I understand it right, before this patch, people couldn't really supply internationalizations for these calls - they would have had to have a translation for each possible value of, e.g. action.choices or parser.prefix_chars.” Exactly.
I will commit this evening and refresh the other patch (there is one overlapping change).
Activity
merwok commentedon Nov 22, 2010
I found two calls to gettext that are incorrect. See attached patch.
abalkin commentedon Nov 22, 2010
The patch makes sense. I wonder if i10n of programs using argparse should be mentioned in docs. Providing a sample translation file somewhere may not be a bad idea at least for testing.
bethard commentedon Nov 24, 2010
Looks like a great fix.
And yes, anyone who knows anything about gettext, please feel free to add a test or ten. ;-) I just copied optparse when I put that stuff in, so I have no confidence in how it's done now.
merwok commentedon Nov 25, 2010
A bit of rationale behind the patch. You probably know that the gettext/_ functions serves two roles: Identifying the strings to translate, and retrieving the translation from a catalog.
At strings extraction time, the string in the line
msg = _('unknown parser %r (choices: %s)' % tup)
will be found by xgettext (just checked), so the .pot and .po files will include it. So far, so good.
At runtime however, the gettext function will get "unknown parser 'parserr' (choices: ('some', 'tuple'))" as argument, which isn’t in the translation catalogs. Doing the string interpolation after the gettext call (my patch) avoids this problem.
I will write a test for those two strings.
merwok commentedon Dec 3, 2010
I’ve started on a test but I’m not sure how to proceed, since I need to use or intercept gettext and compare the error messages of argparse with translated messages to prove that the gettext calls are bogus. There are various ways of using or intercepting gettext from tests, but I have to experiment a bit before I can propose the simplest patch that could possibly work.
Exceptionally, I’d like to commit the change as is, and defer the testing for some months, until I can get to bpo-10529.
bethard commentedon Dec 3, 2010
Yes, I think it's okay to fix this without a test, given that it's a nontrivial amount of work to test gettext stuff. I'd rather have it working now, without tests, than wait until we know how to test stuff with gettext.
It's also a pretty minor, obvious fix. Of course, I assume you'll make sure this also merges correctly with your bpo-10528 patch.
merwok commentedon Dec 3, 2010
From the other bug: “If I understand it right, before this patch, people couldn't really supply internationalizations for these calls - they would have had to have a translation for each possible value of, e.g. action.choices or parser.prefix_chars.” Exactly.
I will commit this evening and refresh the other patch (there is one overlapping change).
merwok commentedon Dec 3, 2010
Fixed in r86984 (py3k) and r86990 (2.7).