-
Notifications
You must be signed in to change notification settings - Fork 131
Make userconfirm prompt strings translatable #2518
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
|
I’ve updated the POT and PO files. I’m not fully sure whether they were supposed to be updated. If not, just let me know and I’ll revert those changes. |
2d02708 to
bb1cc58
Compare
|
CI failure doesn't seem related to my changes. Am I missing something? |
bb1cc58 to
5b433e6
Compare
|
Correct implementation should use nl_langinfo() to retrieve the translated Yes/No expressions. This manual reimplementation is a step in wrong direction. |
|
Agreed, after I learned about https://man7.org/linux/man-pages/man3/nl_langinfo.3.html and |
Thanks for review. I was not aware of nl_langinfo(). |
Thanks for review. |
|
I believe the prompt was intentionally kept noninternationalized because if the correct way needs setting C++ locale, what DNF5 does not do now, you will run into flawed number formatting as tried and described in #1699. Resolving that will first require thorough review all all places where numbers are formatted. |
5b433e6 to
3e5da2c
Compare
I agree with the part that setting the C++ locale will break things, but this patch only uses the already set C locale, and I don't see any number or anything involved that may cause an issue in this patch, so I still can't understand how this patch will break things. I have made the suggested changes using Sorry for being two months late with the update. |
|
Thanks, this does seem to pick up the correct YESEXPR/NOEXPR. However, I still cannot see translations for the "Is this ok [Y/n]: " message, for example, if I add the following to Am I just missing something, were you able to get it working? Another concern is that the yesexpr/noexpr value and the translated message and then if the user types |
Changes:
- Use explicit dgettext("libdnf5-cli", ...) in include/libdnf5-cli/utils/userconfirm.hpp.
- Implement robust regex selection: use nl_langinfo(YESEXPR) if a translation is
found, otherwise force English regex to avoid dangerous mismatches.
- Update CMakeLists.txt to include "dgettext:2" as a translatable keyword.
- Update cmake/Translations.cmake to include userconfirm.hpp in the
libdnf5-cli.pot extraction process.
The prompts can now be localized by translators in the libdnf5-cli
domain. The new logic ensures that if a translation is missing, dnf5
strictly expects English "y/n" input, preventing conflicts with the
system locale's native expressions.
Fixes: rpm-software-management#1868
Assisted-by: Gemini 3 Flash
3e5da2c to
96df84c
Compare
Yes, I was able to get it to work on my local environment. The steps I took were: I ran
Yes, it was a major issue with my previous implementation and I have tried to solve it. Now, the Sorry for the delay with the reply. |
The confirmation prompt "Is this ok [Y/n]:" and response characters were hardcoded strings in the userconfirm.hpp template function, making them untranslatable.
Changes:
The userconfirm template now calls helper functions that return translated strings from the libdnf5-cli translation domain, allowing translators to localize the prompt and response characters.
Fixes: #1868