more server i18n #31
more server i18n #31
Conversation
jokester
commented
Dec 15, 2024
•
jokester
commented
- workaround existing hardcoded string
- server-only gettext variant
- translate language names at server
WalkthroughThis pull request enhances internationalization and localization capabilities throughout the project. Key changes include updates to the language model, translation extraction process, and translation files. A new Changes
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
==========================================
- Coverage 79.75% 78.87% -0.89%
==========================================
Files 109 110 +1
Lines 6689 6765 +76
==========================================
+ Hits 5335 5336 +1
- Misses 1354 1429 +75 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/models/language.py (1)
32-782: Consider moving SYSTEM_LANGUAGES_DATA to a separate configuration file.While the implementation is correct with proper use of
hardcode_textfor all language names, having such a large data structure in the model file affects readability and maintainability.Consider moving this data to a separate configuration file (e.g.,
language_config.py) to improve code organization.app/translations/en/LC_MESSAGES/messages.po (1)
1037-1467: Review and improve translations for clarity.Some translations could be improved for better clarity and consistency:
- "Myanmar (Burmese)" should be just "Burmese" to match other language names
- "Scots Gaelic" should be "Scottish Gaelic" (more commonly used)
- Missing translation for "Language can not be deleted"
Apply these improvements:
-msgstr "Myanmar (Burmese)" +msgstr "Burmese" -msgstr "Scots Gaelic" +msgstr "Scottish Gaelic" msgid "Language can not be deleted" -msgstr "" +msgstr "Language cannot be deleted."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Makefile(2 hunks)app/models/language.py(6 hunks)app/translations/__init__.py(1 hunks)app/translations/en/LC_MESSAGES/messages.po(4 hunks)app/translations/zh/LC_MESSAGES/messages.po(4 hunks)
🔇 Additional comments (7)
Makefile (1)
47-47: LGTM: Pybabel extraction command updated correctly.
The addition of -k hardcode_text to the pybabel extract command ensures that strings wrapped with the new hardcode_text function will be properly extracted for translation.
app/translations/__init__.py (2)
Line range hint 5-6: LGTM: Required imports added.
The imports are correctly added to support the new i18n functionality.
53-59: LGTM: Well-documented helper function for i18n.
The hardcode_text function is properly implemented as a pass-through function to mark strings for translation extraction.
app/models/language.py (4)
12-18: LGTM: Well-structured type definition.
The LanguageData TypedDict provides clear type hints for language data structure, improving code maintainability and type safety.
791-791: LGTM: Properly internationalized logging messages.
The logging messages are correctly wrapped with gettext for internationalization.
Also applies to: 805-805
900-900: LGTM: Internationalized error message.
The error message is properly wrapped with gettext for internationalization.
910-912: LGTM: Improved localized name handling.
The to_api method now correctly handles localized names with a proper fallback to the original name when translation is not available.
| msgid "豪萨语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:292 |
There was a problem hiding this comment.
Review fuzzy translations.
Two translations are marked as fuzzy and need review:
- "弗里斯兰语" (Frisian)
- "印度尼西亚语" (Indonesian)
Please review and confirm these translations, then remove the fuzzy marker if they are correct.
Also applies to: 1203-1206
| msgid "斯洛伐克语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:640 | ||
| msgid "斯洛文尼亚语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:647 | ||
| msgid "索马里语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:654 | ||
| msgid "西班牙语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:661 | ||
| msgid "巽他语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:668 | ||
| msgid "斯瓦希里语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:675 | ||
| msgid "瑞典语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:682 | ||
| msgid "他加禄语（菲律宾语）" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:689 | ||
| msgid "塔吉克语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:696 | ||
| msgid "泰米尔语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:703 | ||
| msgid "泰卢固语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:710 | ||
| msgid "泰语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:717 | ||
| msgid "土耳其语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:724 | ||
| msgid "乌克兰语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:731 | ||
| msgid "乌尔都语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:738 | ||
| msgid "乌兹别克语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:745 | ||
| msgid "越南语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:752 | ||
| msgid "威尔士语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:759 | ||
| msgid "科萨语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:766 | ||
| msgid "意第绪语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:773 | ||
| msgid "约鲁巴语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:780 | ||
| msgid "祖鲁语" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:791 | ||
| msgid "Language collection already initialized." | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:805 | ||
| #, python-format | ||
| msgid "Initialized Language collection with %d languages" | ||
| msgstr "" | ||
|
|
||
| #: app/models/language.py:900 | ||
| msgid "Language can not be deleted" | ||
| msgstr "" | ||
|
|
There was a problem hiding this comment.
Add missing translations for new language entries.
Multiple new language entries have been added but their translations are missing (empty msgstr). This includes entries for Japanese, English, Korean, and many other languages.
Please provide Chinese translations for all new language entries. For example:
msgid "日语"
-msgstr ""
+msgstr "日语"
msgid "英语"
-msgstr ""
+msgstr "英语"Committable suggestion skipped: line range outside the PR's diff.
| msgstr "Project set configuration" | ||
|
|
||
| #: app/models/team.py:110 | ||
| msgid "修改团队内项目集的设置" | ||
| msgstr "Modify the settings of the project collection within the team." | ||
| msgstr "Modify the settings of the project sets within the team." | ||
|
|
||
| #: app/models/team.py:113 | ||
| msgid "删除项目集" | ||
| msgstr "Delete project set" | ||
|
|
||
| #: app/models/team.py:114 | ||
| msgid "删除团队内的项目集" | ||
| msgstr "Delete the portfolio within the team." | ||
| msgstr "Delete the project sets within the team." |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistent punctuation in translations.
Some translations inconsistently use periods at the end. For example:
- "Project set configuration" (no period)
- "Modify the settings of the project sets within the team." (with period)
Apply consistent punctuation:
-msgstr "Project set configuration"
+msgstr "Project set configuration."
-msgstr "Delete project set"
+msgstr "Delete project set."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| msgstr "Project set configuration" | |
| #: app/models/team.py:110 | |
| msgid "修改团队内项目集的设置" | |
| msgstr "Modify the settings of the project collection within the team." | |
| msgstr "Modify the settings of the project sets within the team." | |
| #: app/models/team.py:113 | |
| msgid "删除项目集" | |
| msgstr "Delete project set" | |
| #: app/models/team.py:114 | |
| msgid "删除团队内的项目集" | |
| msgstr "Delete the portfolio within the team." | |
| msgstr "Delete the project sets within the team." | |
| msgstr "Project set configuration." | |
| #: app/models/team.py:110 | |
| msgid "修改团队内项目集的设置" | |
| msgstr "Modify the settings of the project sets within the team." | |
| #: app/models/team.py:113 | |
| msgid "删除项目集" | |
| msgstr "Delete project set." | |
| #: app/models/team.py:114 | |
| msgid "删除团队内的项目集" | |
| msgstr "Delete the project sets within the team." |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
app/translations/__init__.py (2)
56-60: Enhance function documentationWhile the function implementation is correct, the documentation could be more descriptive about its purpose in the i18n workflow.
def hardcode_text(msgid: str) -> str: """ - used to capture hardcoded string as msgid + Marks a hardcoded string for translation extraction. + This function helps identify strings that need to be included + in translation files without modifying the original text. """ return msgid
63-65: Add error handling for locale operationsThe function should handle potential exceptions from
force_localeto ensure robustness.def server_gettext(msgid: str): - with flask_babel.force_locale(_app_config.BABEL_DEFAULT_LOCALE): - return gettext(msgid) + try: + with flask_babel.force_locale(_app_config.BABEL_DEFAULT_LOCALE): + return gettext(msgid) + except Exception as e: + logger.error("Failed to force locale: %s", str(e)) + return gettext(msgid) # Fallback to current localeapp/models/language.py (3)
Line range hint
32-784: Add documentation for language support flagsConsider adding comments to explain the significance of empty
g_tra_codeandg_ocr_codevalues and their impact on functionality.Add this documentation above the SYSTEM_LANGUAGES_DATA:
""" Language support matrix: - g_tra_code: Empty string indicates no Google Translate support - g_ocr_code: Empty string indicates no Google OCR support - no_space: True for languages that don't use spaces between words """
Line range hint
787-805: Enhance initialization loggingConsider adding more detailed logging to help with debugging and monitoring.
if cls.objects.count() > 0: logger.info(server_gettext("Language collection already initialized.")) return sort = 0 for lang in cls.SYSTEM_LANGUAGES_DATA: + logger.debug("Initializing language: %s (%s)", lang["en_name"], lang["code"]) cls( en_name=lang["en_name"], lo_name=lang["lo_name"], no_space=lang.get("no_space", False), code=lang["code"], g_tra_code=lang["g_tra_code"], g_ocr_code=lang["g_ocr_code"], sort=sort, ).save() sort += 1 logger.debug(server_gettext("Initialized Language collection with %d languages"), sort)
910-912: Improve API documentationThe comments explain the behavior but could be formatted as proper documentation.
- # in non-zh locales gettext(hardcoded_id) should return localized name - # otherwise the hardcoded_id is just in zh - "lo_name": gettext(self.lo_name) or self.lo_name, + # Attempts to get the localized name in the current locale. + # Falls back to the original (Chinese) name if no translation exists. + "lo_name": gettext(self.lo_name) or self.lo_name,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/models/language.py(6 hunks)app/translations/__init__.py(2 hunks)
🔇 Additional comments (1)
app/models/language.py (1)
12-18: LGTM! Well-structured type definition
The LanguageData TypedDict provides good type safety and clear structure for language data.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/models/language.py (2)
913-915: Remove redundantorinlo_nameassignmentIn the
to_apimethod, the use ofor self.lo_nameis unnecessary becausegettext(self.lo_name)will return the original string if no translation is found. Removing theorclause simplifies the code without changing its behavior.Simplify the code as follows:
-"lo_name": gettext(self.lo_name) or self.lo_name, +"lo_name": gettext(self.lo_name),
806-808: Adjust logging level for initialization messageThe log message indicating the initialization of the language collection is important for tracking application setup. Using
logger.debugmay result in this message not appearing in production logs. Consider changing the logging level toinfoto ensure it's recorded appropriately.Apply this change:
-logger.debug( +logger.info( server_gettext("Initialized Language collection with %d languages"), sort )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/models/language.py(6 hunks)app/translations/en/LC_MESSAGES/messages.po(5 hunks)app/translations/zh/LC_MESSAGES/messages.po(4 hunks)
🔇 Additional comments (3)
app/translations/en/LC_MESSAGES/messages.po (1)
1717-1729: Ensure consistent punctuation in translations
Previously, there was an inconsistency in using periods at the end of translations. It appears that this issue has been addressed, and the translations now maintain consistent punctuation.
app/translations/zh/LC_MESSAGES/messages.po (2)
1146-1149: Review and confirm fuzzy translations
Previously, translations for "弗里斯兰语" (Frisian) and "印度尼西亚语" (Indonesian) were marked as fuzzy and required review. Please ensure these translations are accurate and remove any fuzzy markers if they are correct.
To verify, please check the following entries:
#: app/models/language.py:235
msgid "弗里斯兰语"
-msgstr ""
+msgstr "弗里斯兰语"
#: app/models/language.py:341
msgid "印度尼西亚语"
-msgstr ""
+msgstr "印度尼西亚语"Also applies to: 1178-1181
1011-1444:
Add missing Chinese translations for new language entries
Several new language entries have been added with empty msgstr values. These entries require Chinese translations to ensure proper localization. This includes languages such as "日语" (Japanese), "英语" (English), "韩语" (Korean), and many others.
Please provide the Chinese translations for these entries. For example:
msgid "日语"
-msgstr ""
+msgstr "日语"
msgid "英语"
-msgstr ""
+msgstr "英语"
msgid "韩语"
-msgstr ""
+msgstr "韩语"Likely invalid or redundant comment.
| "lo_name": hardcode_text("韩语") or hardcode_text("朝鲜语（韩语）"), | ||
| "code": "ko", | ||
| "g_tra_code": "ko", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify 'lo_name' assignments by removing redundant 'or' operators
The use of the or operator between two hardcode_text function calls in the lo_name assignments may not produce the intended effect. Since hardcode_text returns a non-empty string, the expression after the or will never be evaluated. If the goal is to provide a fallback or alternative, consider using a list or handling this logic elsewhere.
Apply the following changes to simplify the assignments:
-"lo_name": hardcode_text("韩语") or hardcode_text("朝鲜语（韩语）"),
+"lo_name": hardcode_text("韩语"),
-"lo_name": hardcode_text("马拉地语") or hardcode_text("马拉语"),
+"lo_name": hardcode_text("马拉地语"),
-"lo_name": hardcode_text("齐切瓦语 (尼扬贾语)") or hardcode_text("齐切瓦语"),
+"lo_name": hardcode_text("齐切瓦语 (尼扬贾语)"),
-"lo_name": hardcode_text("斯洛文尼亚语") or hardcode_text("斯卡洛文语"),
+"lo_name": hardcode_text("斯洛文尼亚语"),Also applies to: 485-486, 521-522, 641-642
| @@ -888,7 +900,7 @@ | |||
|
|
|||
| def clear(self) -> None: | |||
| """禁止删除语言""" | |||
| pass | |||
| raise AssertionError(gettext("Language can not be deleted")) | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a more appropriate exception than AssertionError
Raising an AssertionError to prevent deletion is not ideal, as assertions are generally used for debugging purposes and may be ignored when optimizations are applied. Instead, consider raising a custom exception or a more suitable built-in exception like NotImplementedError to indicate that the deletion operation is not allowed.
Apply this change:
-def clear(self) -> None:
- raise AssertionError(gettext("Language can not be deleted"))
+def clear(self) -> None:
+ raise NotImplementedError(gettext("Language cannot be deleted"))Committable suggestion skipped: line range outside the PR's diff.
