Skip to content
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

gh-77956: Add the words 'default' and 'version' help text localizable #12711

Merged
merged 4 commits into from
Feb 26, 2024

Conversation

eamanu
Copy link
Contributor

@eamanu eamanu commented Apr 7, 2019

bpo-33775: argparse: the word 'default' (in help) is not marked as translatable

and bpo-16786: argparse doesn't offer localization interface for "version" action

Co-authored-by: paul.j3

@eamanu eamanu changed the title Add the words 'default' and 'version' help text localizable. bpo-33775: Add the words 'default' and 'version' help text localizable. Apr 7, 2019
Copy link
Member

@tirkarthi tirkarthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs tests and I am not sure how to test this since argparse lacks localization tests. As noted by paul.j3 I am not sure how worthy enough it is to make this change that doesn't have tests and something the user can override by having a custom help formatter.

Wrapping the whole help string at https://github.com/python/cpython/pull/12711/files#diff-837b312b1f3508216ace6adb46492836R682 will cause %(default)s also to be changed as a translated variable name causing KeyError. I think the issue was to just make word default in help translatable.

Sample program where I directly patch gettext so that translation just gives uppercase of the word :

import gettext

def my_gettext(s):
    return s.upper()

gettext.gettext = my_gettext

import argparse

if __name__ == "__main__":
    parser = argparse.ArgumentParser(formatter_class=argparse.ArgumentDefaultsHelpFormatter)
    parser.add_argument("-a", default=1, help= "Test help")
    args = parser.parse_args()
python3 ../backups/bpo33775.py --help
USAGE: bpo33775.py [-h] [-a A]

OPTIONAL ARGUMENTS:
  -h, --help  SHOW THIS HELP MESSAGE AND EXIT
  -a A        Test help (default: 1)

with patch %(default)s is changed to %(DEFAULT)s:

./python.exe ../backups/bpo33775.py --help
Traceback (most recent call last):
  File "../backups/bpo33775.py", line 12, in <module>
    args = parser.parse_args()
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 1746, in parse_args
    args, argv = self.parse_known_args(args, namespace)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 1778, in parse_known_args
    namespace, args = self._parse_known_args(args, namespace)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 1984, in _parse_known_args
    start_index = consume_optional(start_index)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 1924, in consume_optional
    take_action(action, args, option_string)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 1852, in take_action
    action(self, namespace, argument_values, option_string)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 1034, in __call__
    parser.print_help()
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 2471, in print_help
    self._print_message(self.format_help(), file)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 2455, in format_help
    return formatter.format_help()
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 281, in format_help
    help = self._root_section.format_help()
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 212, in format_help
    item_help = join([func(*args) for func, args in self.items])
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 212, in <listcomp>
    item_help = join([func(*args) for func, args in self.items])
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 212, in format_help
    item_help = join([func(*args) for func, args in self.items])
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 212, in <listcomp>
    item_help = join([func(*args) for func, args in self.items])
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 522, in _format_action
    help_text = self._expand_help(action)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/argparse.py", line 611, in _expand_help
    return self._get_help_string(action) % params
KeyError: 'DEFAULT'

@brettcannon brettcannon added the type-bug An unexpected behavior, bug, or error label Apr 17, 2019
@jdetrey
Copy link
Contributor

jdetrey commented Aug 8, 2021

Dear all,

My two cents about this issue, as I would also like to see this PR merged :)

Wrapping the whole help string at https://github.com/python/cpython/pull/12711/files#diff-837b312b1f3508216ace6adb46492836R682 will cause %(default)s also to be changed as a translated variable name causing KeyError. I think the issue was to just make word default in help translatable.

I disagree with this comment: gettext translations should never change named arguments in format strings. Named arguments are actually recommended in the GNU gettext documentation (see the note at the end of the Python section).

Therefore, passing the whole string to _() (as done by this PR) is the way to go. It is also consistent with other calls to _() in the argparse module (see e.g. Lib/argparse.py:2212).

Kind regards,
Jérémie.

Lib/argparse.py Outdated Show resolved Hide resolved
jdetrey added a commit to jdetrey/cpython that referenced this pull request Aug 8, 2021
A couple of other missing translations are addressed by python#12711.

Incidentally, this also fixes a bug with the "(default: …)" help
string of the `BooleanOptionalAction` class: when used with the
`ArgumentDefaultsHelpFormatter`, a second "(default: …)" string
would be added to the help text. This was due to the fact that
the `default` value was immediately expanded in the help string,
and thus wasn't detected as a "(default: …)" string by the
`ArgumentDefaultsHelpFormatter._get_help_string()` method
(which relies on finding a named format argument `%(default)`
inside the help string).
@arhadthedev
Copy link
Member

@tirkarthi argparse already has a bunch of untested translatable strings so the PR is not a pioneer here:

prefix = _('usage: ')
format = _('argument %(argument_name)s: %(message)s')
raise NotImplementedError(_('.__call__() not defined'))
self, _('conflicting subparser alias: %s') % alias)
msg = _('unknown parser %(parser_name)r (choices: %(choices)s)') % args

and way more.

@erlend-aasland erlend-aasland changed the title bpo-33775: Add the words 'default' and 'version' help text localizable. gh-77956: Add the words 'default' and 'version' help text localizable Jan 12, 2024
Copy link
Contributor

@m-aciek m-aciek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@serhiy-storchaka
Copy link
Member

@tirkarthi, I agree with @jdetrey and @arhadthedev. Translation should not change %(default)s. Adding translation tests to argparse is a separate and more complex issue, this should not stop all bugfixes.

@tirkarthi tirkarthi dismissed their stale review February 26, 2024 17:57

Stale review comment

@tirkarthi
Copy link
Member

@serhiy-storchaka I have removed my request for changes since it was an old review. I can see argparse already has this approach as noted. Please go ahead with your review. Thanks.

@serhiy-storchaka serhiy-storchaka enabled auto-merge (squash) February 26, 2024 18:04
@serhiy-storchaka serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Feb 26, 2024
@serhiy-storchaka serhiy-storchaka merged commit da382aa into python:main Feb 26, 2024
35 checks passed
@miss-islington-app
Copy link

Thanks @eamanu for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @eamanu and @serhiy-storchaka, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker da382aaf52d761a037874bee27bb5db69906df9e 3.12

@miss-islington-app
Copy link

Sorry, @eamanu and @serhiy-storchaka, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker da382aaf52d761a037874bee27bb5db69906df9e 3.11

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Feb 26, 2024
…t localizable (pythonGH-12711)

(cherry picked from commit da382aa)

Co-authored-by: Emmanuel Arias <[email protected]>
Co-authored-by: paul.j3
Co-authored-by: Jérémie Detrey <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 26, 2024

GH-115967 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Feb 26, 2024
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Feb 26, 2024
…t localizable (pythonGH-12711)

(cherry picked from commit da382aa)

Co-authored-by: Emmanuel Arias <[email protected]>
Co-authored-by: paul.j3
Co-authored-by: Jérémie Detrey <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 26, 2024

GH-115968 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Feb 26, 2024
serhiy-storchaka added a commit that referenced this pull request Feb 26, 2024
…lizable (GH-12711) (GH-115967)

(cherry picked from commit da382aa)

Co-authored-by: paul.j3
Co-authored-by: Emmanuel Arias <[email protected]>
Co-authored-by: Jérémie Detrey <[email protected]>
serhiy-storchaka added a commit that referenced this pull request Feb 26, 2024
…lizable (GH-12711) (GH-115968)

(cherry picked from commit da382aa)

Co-authored-by: paul.j3
Co-authored-by: Emmanuel Arias <[email protected]>
Co-authored-by: Jérémie Detrey <[email protected]>
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
…izable (pythonGH-12711)

Co-authored-by: paul.j3
Co-authored-by: Jérémie Detrey <[email protected]>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
…izable (pythonGH-12711)

Co-authored-by: paul.j3
Co-authored-by: Jérémie Detrey <[email protected]>
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
…izable (pythonGH-12711)

Co-authored-by: paul.j3
Co-authored-by: Jérémie Detrey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants