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

I18N: Annotate literals in advisory command #1161

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

ppisar
Copy link
Contributor

@ppisar ppisar commented Jan 12, 2024

The comment is extracted by "xgettext -c" and placed to gettext catalogs. It helps the translators not know which words not to translate:

#. Note for translators: "critical" etc. quoted words are
#. literals that should not be translated.
#: commands/advisory/arguments.hpp:155
msgid ""
"Limit to packages in advisories with specified severity. List option. Can be "
"\"critical\", \"important\", \"moderate\", \"low\", \"none\"."
msgstr ""

Fixes: #1159

The comment is extracted by "xgettext -c" and placed to gettext
catalogs. It helps the translators not know which words not to
translate:

    #. Note for translators: "critical" etc. quoted words are
    #. literals that should not be translated.
    #: commands/advisory/arguments.hpp:155
    msgid ""
    "Limit to packages in advisories with specified severity. List option. Can be "
    "\"critical\", \"important\", \"moderate\", \"low\", \"none\"."
    msgstr ""

Fixes: rpm-software-management#1159
@rffontenelle
Copy link
Contributor

How about making them placeholders {} instead?

@j-mracek
Copy link
Contributor

LGTM

@ppisar
Copy link
Contributor Author

ppisar commented Jan 16, 2024

Removing the literals from the message and substituting them dynamically would be bad because:

  • The message would have to be built at run-time. A static string as it is now is much faster and saves a memory.
  • The data structure the message is stored now does not support run-time computations. The text must be ready when compiling the code. Using a formating string would require changing the code.
  • The translators could not explain a meaning of the literals in their translations, like:
msgid ""
"Limit to packages in advisories with specified severity. List option. Can be "
"\"critical\", \"important\", \"moderate\", \"low\", \"none\"."
msgstr ""
"Limit to packages in advisories with specified severity. List option. Can be "
"\"critical\" [very serious], \"important\" [serious], \"moderate\" [normal] , \"low\" [unimportant], \"none\" [undefined]."
~~~

Here in English to English translation it does not highlight the problem boldly, but I believe you know what I mean.

@@ -150,6 +150,8 @@ class AdvisorySeverityOption : public libdnf5::cli::session::AppendStringListOpt
command,
"advisory-severities",
'\0',
/* Note for translators: "critical" etc. quoted words are
literals that should not be translated. */
_("Limit to packages in advisories with specified severity. List option. Can be "
Copy link
Contributor

@j-mracek j-mracek Jan 17, 2024

Choose a reason for hiding this comment

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

I am curios whether we should use C_() to add a context for translators instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Translation contexts are for distinguishing two identical messages for a need of different translations. This is not the case. This message only exists at two places dnf5/commands/advisory_shared.hpp and dnf5daemon-client/commands/advisory/arguments.hpp. The messages has the same meaning and use (their code and method name is actually a duplicate), so no artificial context is needed here.

@j-mracek j-mracek added this pull request to the merge queue Jan 17, 2024
Merged via the queue into rpm-software-management:main with commit 98c8612 Jan 17, 2024
5 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Make dnf5daemon-client values placeholder instead of hardcoding them
3 participants