-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Localisation of more strings #2902
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
base: main
Are you sure you want to change the base?
Conversation
Babel does not support f-strings, which prevents from localising a few strings in the code, like "Usage: " and "Try". The changes in this commit rewrites f-strings into the format syntax. Please note some f-strings remain as they are not expected to be translatable. This commit has to deactivate the UP032 rule on a few files. There seems to be an unreported bug in either Ruff or pyupgrade: The UP032 rule is not deactivated on multi-line commands, including multi-line strings.
Babel does not support f-strings, which prevents from localising a few strings in the code, like "Usage: " and "Try". The changes in this commit rewrites f-strings into the format syntax. Please note some f-strings remain as they are not expected to be translatable. This commit has to deactivate the UP032 rule on a few files. There seems to be an unreported bug in either Ruff or pyupgrade: The UP032 rule is not deactivated on multi-line commands, including multi-line strings.
Please note Windows may requires extra configuration (as it may not set variables expected by gettext). Click does not perform the extra for some reason, and deemed out of scope.
Some f-strings changed to the format method in earlier commits do not need localisation. This commit restores them to reduce the amount of changes.
|
I don't think we are willing to limit the use of f strings in the project at this time. Thoughts @davidism? |
| # always force f-strings. The latter are unfortunately not supported yet | ||
| # by Babel, a localisation library. | ||
| # | ||
| # Note: Using `# noqa: UP032` on lines has not worked, so a file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of these noqa marks are needed. Ruff is not trying to autoupgrade anything if I remove the comments, inline or file-level.
| f"It is not possible to add the group {cmd_name!r} to another" | ||
| f" group {base_command.name!r} that is in chain mode." | ||
| ) | ||
| message = _( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're only translating user-facing messages at this time, not developer-facing. Please remove all such translation markings.
|
For user-facing messages, yes we can use the The We're only translating user-facing messages at this time, not developer-facing. Please remove all such translation markings. |
| else "(DEPRECATED)" | ||
| else _("(DEPRECATED)") | ||
| ) | ||
| text = _("{text} {deprecated_message}").format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't have ever been marked for translation. It should be removed, since you've added the translation to the proper location above.
|
Most of the changes here need to be rolled back, they are not part of the user-facing output. |
|
Hey thanks for the review and feedback. A quick note to let you know I expect to improve the work here soon, most likely in November. Sorry for anyone watching and expecting this sooner. |
| return "{d}{day_label} {h:02}:{m:02}:{s:02}".format( | ||
| d=t, | ||
| day_label=_("d"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return "{d}{day_label} {h:02}:{m:02}:{s:02}".format( | |
| d=t, | |
| day_label=_("d"), | |
| return "{d}{day_label} {h:02}:{m:02}:{s:02}".format( | |
| d=t, | |
| # TRANSLATORS: The 'd' stands for 'day'. | |
| day_label=_("d"), |
Problem and proposal
Using Click in a multi-lingual package, we would like to localise more strings than currently possible.
In our work before this PR, we reach like:
The package we develop yields the
artefactscommand, based on Click, works alright to translate our strings, as well as a range of strings in Click wrapped withgettext.With this PR, we get:
Which looks like possibly full coverage of meaningful strings in Click.
Note this PR addresses two issues:
gettextLimitation, side-effects and discussion
It looks like part of this PR may be desired, as localisation is already in place---this mainly covers more of the strings (I'd say all the strings, but localisation does not always make sense).
However in the current approach I had to "deal" with f-strings and PyUpgrade, which may be undesired change for the project. On top of that it seems there is a bug in either Ruff or PyUpgrade in accepting ignore rules on multi-line commands. So a bunch of files get a file-global deactivation of the U032 rule, which means these files will not get checked for f-strings.
F-strings are assumedly desired, but given (1) PyBabel does not and may not support them, and (2) Click is library code, the project may (have to) accept the format method everywhere localisation is needed.
Recent discussions mention f-strings may work in PyBabel from Python 3.12. We did not confirm it, as we want to support down to the oldest supported Python3. So the changed proposed here may be transient for a couple years (then PyUpgrade may be reactivated and let work).
Related work
This PR only aims at localising more strings, so related to i18n issues.
On the way to this PR, we have considered a couple alternatives, notably trying to use the class API of Python's
gettext. Some elements of discussion here may be useful to:In fact, Carmen's post helped solve an issue in using catalogues from different domains at runtime (thanks!).
Checklist on CONTRIBUTING
.. versionchanged::entries in any relevant code docs.At submission time, nothing checked here, as first would like to make sure this PR target is acceptable (likely not as-is). The tox-based checks all pass, though (i.e. running the
toxcommand returns all green, except the skipped tests).This PR supersedes #2890, because of a problem with GitHub.