gh-130453: pygettext: Allow specifying multiple keywords with the same function name #131380
Conversation
Tools/i18n/pygettext.py
Outdated
| f'arguments are not allowed in gettext calls', file=sys.stderr) | ||
| return False | ||
| return (f'*** {self.filename}:{node.lineno}: Variable positional ' | ||
| f'arguments are not allowed in gettext calls') |
There was a problem hiding this comment.
| f'arguments are not allowed in gettext calls') | |
| f'arguments are not allowed in gettext calls') |
There was a problem hiding this comment.
This error will still be printed multiple times.
Tools/i18n/pygettext.py
Outdated
| return (f'*** {self.filename}:{node.lineno}: Expected at least ' | ||
| f'{max_index + 1} positional argument(s) in gettext call, ' | ||
| f'got {len(node.args)}') |
There was a problem hiding this comment.
With --keyword=_:1 --keyword=_:2, multiple errors will be printed for _(): "Expected at least 1 positional argument(s)...", "Expected at least 2 positional argument(s)...".
Lib/test/test_tools/test_i18n.py
Outdated
| b'*** test.py:1: Expected a string constant for argument 1, got x\n' | ||
| b'*** test.py:1: Expected a string constant for argument 2, got 42\n' |
There was a problem hiding this comment.
This is misleading, since only one of arguments needs to be a string.
Indeed :/ I'm still trying to figure out the best way to show the messages. I'll make the PR a draft for now while I experiment with it |
|
Ok so I think the main issue with the error messages is that it is not clear that pygettext is trying to match multiple specs and each of the error message relates to a different spec but for the same function call. I think we can make the message more structured in case there are multiple specs for the same function name. Something like this: The 'main' message makes it clear there are multiple specs and none of them matched. Then each sub-message is prefixed with a particular spec and shows the error for that spec. This is in my opinion clearer and makes it easier for the user to figure out where the problem is. |
|
What |
|
xgettext does not print anything when it can't extract a message, which is also an option I suppose, though pygettext has had these error messages since the beginning so I don't know if it's a good idea to get rid of them? |
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
I implemented the example I showed above. It collects errors from all keyword specs for a given func name and then prints them all in a structured way that is hopefully both clear and helpful. I'd like to know what you think! Alternatively, we could just get rid of the error messages which would simplify the code. Though as I wrote before, pygettext has always had these error messages and I think they are useful as they help you find i18n issues in your code. |
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…he same function name (pythonGH-131380)
There has been some recent work making the custom keyword option (
--keyword) more useful and flexible (#130463 & #130709)One thing that is still missing, is the ability to specify multiple keywords with the same function name. For example
python pygettext.py --keyword=foo --keyword=foo:1,2. This should extract calls likefoo('bar')as regular gettext and calls likefoo('bar', 'bars', 1)as ngettext.Specifying multiple keywords is already supported by both xgettext and babel and it is in general quite useful to be able to do this for cases when you have an overloaded gettext function which behaves differently based on the number of arguments passed to it.
This PR allows you to pass multiple keywords with the same function name, for example
--keyword=foo --keyword=foo:1,2is allowed and does what one would expect.This does not let you distinguish between e.g.
ngettextandpgettextsince there is no way to specify the total number of arguments which is done with thetspecifier (e.g.foo:1c,2,2tfor pgettext andfoo:1,2,3tfor ngettext). This can be added later.Feedback welcome :)