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

POT Generator: Add support for TRANSLATORS: and NO_TRANSLATE comments #98099

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

dalexeev
Copy link
Member

@dalexeev dalexeev commented Oct 11, 2024

Example
extends Node

func _ready() -> void:
    print(tr("tr1"))
    print(tr("tr2", "ctx_a"))
    print(tr("tr3")) # NO_TRANSLATE
    print(tr("tr4")) # TRANSLATORS: Message 4
    # TRANSLATORS: Message 5
    print(tr("tr5"))
    # Above comment.
    # TRANSLATORS: Message 6 Line 1
    # Message 6 Line 2
    # Message 6 Line 3
    print(tr("tr6"))
    # Above comment.
    # TRANSLATORS: Message 7 Line 1
    # Message 7 Line 2
    # Message 7 Line 3
    print(tr_n("tr_n7", "tr_n7_plural", 0, "ctx_b"))

    print(tr("tr8", "ctx_c")) # TRANSLATORS: Message 8.1
    print(tr("tr8", "ctx_c")) # TRANSLATORS: Message 8.1
    print(tr("tr8", "ctx_c")) # TRANSLATORS: Message 8.2
    print(tr("tr8", "ctx_d")) # TRANSLATORS: Message 8.3

    # TRANSLATORS: Message 9

    # Empty line ^^
    print(tr("tr9"))

    print(tr("tr10")) # TRANSLATORS: Message 10
    print(tr("tr11"))
# LANGUAGE translation for temp for the following files:
# res://node.gd
#
# FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.
#
#, fuzzy
msgid ""
msgstr ""
"Project-Id-Version: temp\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8-bit\n"

#: node.gd
msgid "tr1"
msgstr ""

#: node.gd
msgctxt "ctx_a"
msgid "tr2"
msgstr ""

#. TRANSLATORS: Message 4
#: node.gd
msgid "tr4"
msgstr ""

#. TRANSLATORS: Message 5
#: node.gd
msgid "tr5"
msgstr ""

#. TRANSLATORS: Message 6 Line 1
#. Message 6 Line 2
#. Message 6 Line 3
#: node.gd
msgid "tr6"
msgstr ""

#. TRANSLATORS: Message 7 Line 1
#. Message 7 Line 2
#. Message 7 Line 3
#: node.gd
msgctxt "ctx_b"
msgid "tr_n7"
msgid_plural "tr_n7_plural"
msgstr[0] ""
msgstr[1] ""

#. TRANSLATORS: Message 8.1
#. Message 8.2
#: node.gd
msgctxt "ctx_c"
msgid "tr8"
msgstr ""

#. TRANSLATORS: Message 8.3
#: node.gd
msgctxt "ctx_d"
msgid "tr8"
msgstr ""

#: node.gd
msgid "tr9"
msgstr ""

#. TRANSLATORS: Message 10
#: node.gd
msgid "tr10"
msgstr ""

#: node.gd
msgid "tr11"
msgstr ""

@AdriaandeJongh
Copy link
Contributor

AdriaandeJongh commented Oct 11, 2024

Just curious, because it's missing from the examples: what happens to comments surrounding a single line with 2 tr()'s? Eg:

# TRANSLATORS: hello
print(tr("key_here"), tr("another_key"))

And when there's both a comment above and behind?

# TRANSLATORS: hello
print(tr("key_here"), tr("another_key")) # TRANSLATORS: say what

@dalexeev
Copy link
Member Author

what happens to comments surrounding a single line with 2 tr()'s? Eg:

# TRANSLATORS: hello
print(tr("key_here"), tr("another_key"))

This will be applied to both strings, the parser does not check if the comment is consumed. But you can write:

print(
    tr("key_here"), # TRANSLATORS: hello
    tr("another_key"), # TRANSLATORS: world
)

or:

print(
    # TRANSLATORS: hello
    tr("key_here"),
    # TRANSLATORS: world
    tr("another_key"),
)

And when there's both a comment above and behind?

# TRANSLATORS: hello
print(tr("key_here"), tr("another_key")) # TRANSLATORS: say what

Inline comment takes precedence, comment above will not be used.

@dalexeev dalexeev force-pushed the pot-gen-add-comment-support branch 2 times, most recently from b2b6514 to c37d2f3 Compare October 11, 2024 20:20
@timothyqiu
Copy link
Member

In your example,

    print(tr("tr8", "ctx_c")) # TRANSLATORS: Message 8.1
    print(tr("tr8", "ctx_c")) # TRANSLATORS: Message 8.2

generates

#. TRANSLATORS: Message 8.1
#. TRANSLATORS: Message 8.2
#: node.gd
msgctxt "ctx_c"
msgid "tr8"
msgstr ""

This makes the translators comment

Message 8.1
TRANSLATORS: Message 8.2

The expected result is

Message 8.1
Message 8.2

@dalexeev
Copy link
Member Author

@timothyqiu I wasn't sure if we need remove the TRANSLATORS: prefix, so I left it like in Godot editor translations.

https://github.com/godotengine/godot-editor-l10n/blob/b8f7a2d06918f897eed69f7d481438bc48ce7595/editor/editor.pot#L1091-L1094

@timothyqiu
Copy link
Member

    print(tr("tr8", "ctx_c")) # TRANSLATORS: Message 8.1
    print(tr("tr8", "ctx_c")) # TRANSLATORS: Message 8.2

@dalexeev I mean, the message tr8 effectively has is a multiline comment and the second TRANSLATORS: is currently part of the comment body (the first TRANSLATORS: can be recognized by tools like Poedit).

Perhaps the TRANSLATORS: prefix should be kept on the first line only. Maybe also generate a blank line between the two lines, since they come from different code blocks.

@dalexeev
Copy link
Member Author

@timothyqiu Done, updated the example.

@Vovkiv
Copy link

Vovkiv commented Oct 12, 2024

Great job!
I have one suggestion, that probably should be made in separate PR, but what about adding "TRANSLATORS:" as default theme marker in editor settings?

Знімок екрана з 2024-10-12 23-30-29

Maybe as notice perhaps?

@dalexeev
Copy link
Member Author

I think we should have a separate color for built-in comment keywords to distinguish them from user ones. I have a local branch for GDScriptSyntaxHighlighter refactoring, where I want to add highlighting of BBCode markup and special tags (@tutorial, @deprecated, @experimental) in doc comments. The same for TRANSLATORS and NO_TRANSLATE.

Copy link
Member

@timothyqiu timothyqiu left a comment

Choose a reason for hiding this comment

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

Tested some common use cases locally, and it worked as expected.

@dalexeev
Copy link
Member Author

Fixed a case I forgot about (non-inline comment should only be taken into account if there is no code in that line).

print(tr("tr10")) # TRANSLATORS: Message 10
print(tr("tr11"))

@molingyu
Copy link

07f76bf8923795a44f3b7a3756c3d809

Should we add a comment property under the Localization property group in the UI system?

@timothyqiu
Copy link
Member

timothyqiu commented Oct 13, 2024

Should we add a comment property under the Localization property group in the UI system?

This falls into the same category as translation context: godotengine/godot-proposals#8061

Currently, there are several one-to-many / many-to-one situations that need to be sorted out. I don't think node's auto translate system is ready for this. At least it should be a separate PR, it's not very GDScript related :P

@molingyu
Copy link

@timothyqiu Yes, it is indeed not very relevant to this PR.

I want to reset the translation content generation process of the entire UI system, so that it can automatically collect the marked multilingual information from tscn.

@akien-mga akien-mga removed this from the 4.x milestone Nov 14, 2024
@akien-mga akien-mga added this to the 4.4 milestone Nov 14, 2024
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks great!

@YeldhamDev
Copy link
Member

Does this also works for atr and atr_n? Also, this should really be documented somewhere, either in the docs, or in the (a)tr(_n) descriptions. Otherwise, it becomes those kind of features that exist but no one knows about.

@dalexeev
Copy link
Member Author

dalexeev commented Nov 15, 2024

Does this also works for atr and atr_n?

Also, this should really be documented somewhere, either in the docs, or in the (a)tr(_n) descriptions.

I thought we would add this to the online documentation, but maybe we should add it to the class reference as well. However, I'm concerned that this is only useful for POT users, and is useless for CSV. Also, the plugin recognizes many patterns, TRANSLATORS: and NO_TRANSLATE apply to all of them, not just (a)tr(_n).

@akien-mga
Copy link
Member

I don't see a natural place to include it in the class reference indeed, but there should be a section about translator comments added to https://docs.godotengine.org/en/stable/tutorials/i18n/localization_using_gettext.html

Then maybe we can also ensure this page or the whole i18n section is properly linked in relevant parts of the classref, like maybe the tr description at least.

@Mickeon
Copy link
Contributor

Mickeon commented Nov 15, 2024

I don't know if a thumbs-up gives it justice but it's my thoughts exactly.
This information should not be in the class reference, but the page about it should definitely be mentioned in tr() & co. At least, a class reference link to another class that specialises in translations.

@Repiteo Repiteo merged commit e9ce393 into godotengine:master Nov 15, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 15, 2024

Thanks!

@dalexeev dalexeev deleted the pot-gen-add-comment-support branch November 15, 2024 16:53
@AThousandShips
Copy link
Member

AThousandShips commented Nov 15, 2024

I completely missed this PR but this needs some fixes to handle extensions due to the fact that we cannot pass arrays and data to extensions, see:

So this will need a fix for these cases as well for 4.4 or these won't work as far as I can tell

Edit: Haven't confirmed but this might have been fixed in:

But made a PR for changing the signature of this in any case to ensure proper function, especially as it should only be partially "fixed" (you need to const_cast the arguments as they're passed as const &)

@AThousandShips
Copy link
Member

I'll write up a dedicated fix for just these methods to get it in quickly

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