Skip to content

Conversation

@ic
Copy link

@ic ic commented May 12, 2025

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:

> export LANG=ja_JP.UTF-8
> artefacts hello
Usage: artefacts hello [OPTIONS] プロジェクト名
Try 'artefacts hello --help' for help.

Error: Missing argument 'プロジェクト名'.

The package we develop yields the artefacts command, based on Click, works alright to translate our strings, as well as a range of strings in Click wrapped with gettext.

With this PR, we get:

> export LANG=ja_JP.UTF-8
> artefacts hello
使用法:  artefacts hello [OPTIONS] プロジェクト名
ヘルプについては 'artefacts hello --help' を試してください。

エラー:引数がありません 'プロジェクト名'.

Which looks like possibly full coverage of meaningful strings in Click.

Note this PR addresses two issues:

  1. Several strings where not wrapped into gettext
  2. Libraries like PyBabel (most users?) cannot work with f-strings, so the pathological approach to use the format method, and to deactivate the PyUpgrade U032 rule (converts to f-strings).

Limitation, 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

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. 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 tox command returns all green, except the skipped tests).

This PR supersedes #2890, because of a problem with GitHub.

ic added 7 commits April 30, 2025 14:54
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.
@Rowlando13
Copy link
Collaborator

I don't think we are willing to limit the use of f strings in the project at this time. Thoughts @davidism?

@Rowlando13 Rowlando13 marked this pull request as draft August 23, 2025 08:24
# 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
Copy link
Member

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 = _(
Copy link
Member

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.

@davidism
Copy link
Member

For user-facing messages, yes we can use the _("").format() pattern, f-strings can't be used for translation.

The # noqa: UP036 mark does not seem to be needed, please remove this.

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(
Copy link
Member

@davidism davidism Aug 23, 2025

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.

@davidism
Copy link
Member

Most of the changes here need to be rolled back, they are not part of the user-facing output.

@ic
Copy link
Author

ic commented Oct 15, 2025

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.

Comment on lines +177 to +179
return "{d}{day_label} {h:02}:{m:02}:{s:02}".format(
d=t,
day_label=_("d"),

Choose a reason for hiding this comment

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

Suggested change
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"),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants