update i18n script && fix Korean display bug#146
Conversation
WalkthroughThis update standardizes language handling in the firmware. It replaces variable-based language counts with fixed constants, introduces clear enums for languages, and centralizes language string access through a unified table. Several UI strings are updated to say "Confirm Passphrase" instead of "Use This Passphrase?" across all supported languages. The code also gains new scripts to auto-generate language headers and sources. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Firmware
participant LanguageTable
User->>Firmware: Selects or sets language
Firmware->>LanguageTable: Looks up language index/strings using I18N_LANGUAGE_ITEMS and enums
LanguageTable-->>Firmware: Returns localized string
Firmware-->>User: Displays correct localized message (e.g., "Confirm Passphrase")
sequenceDiagram
participant Script
participant i18n.py
participant HeaderFile
participant SourceFile
Script->>i18n.py: Run localization export
i18n.py->>HeaderFile: Generate i18n.h with enums, constants, externs
i18n.py->>SourceFile: Generate i18n.c with language arrays and table
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (3.3.7)legacy/script/i18n.py📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (13)
🧰 Additional context used🧬 Code Graph Analysis (2)legacy/firmware/fsm_msg_nostr.h (2)
legacy/firmware/trezor.c (2)
🪛 Ruff (0.11.9)legacy/script/i18n.py48-48: Missing return type annotation for public function Add return type annotation: (ANN201) 48-48: Missing type annotation for function argument (ANN001) 48-48: Unused function argument: (ARG001) 48-48: Missing type annotation for function argument (ANN001) 74-74: Trailing comma missing Add trailing comma (COM812) 86-86: Trailing comma missing Add trailing comma (COM812) 93-93: Missing return type annotation for public function Add return type annotation: (ANN201) 93-93: Missing type annotation for function argument (ANN001) 93-93: Unused function argument: (ARG001) 126-126: Trailing comma missing Add trailing comma (COM812) 137-137: Trailing comma missing Add trailing comma (COM812) 148-148: Trailing comma missing Add trailing comma (COM812) 159-159: Trailing comma missing Add trailing comma (COM812) ⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (12)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (18)
legacy/firmware/config.c(2 hunks)legacy/firmware/config_emu.c(2 hunks)legacy/firmware/gettext.c(1 hunks)legacy/firmware/i18n/i18n.c(2 hunks)legacy/firmware/i18n/i18n.h(2 hunks)legacy/firmware/i18n/keys.h(1 hunks)legacy/firmware/i18n/locales/de.inc(1 hunks)legacy/firmware/i18n/locales/en.inc(1 hunks)legacy/firmware/i18n/locales/es.inc(1 hunks)legacy/firmware/i18n/locales/ja.inc(1 hunks)legacy/firmware/i18n/locales/ko_kr.inc(1 hunks)legacy/firmware/i18n/locales/pt_br.inc(1 hunks)legacy/firmware/i18n/locales/zh_cn.inc(2 hunks)legacy/firmware/i18n/locales/zh_tw.inc(1 hunks)legacy/firmware/layout2.c(4 hunks)legacy/firmware/menu_core.c(1 hunks)legacy/firmware/trezor.c(1 hunks)legacy/script/i18n.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
legacy/firmware/trezor.c (2)
legacy/firmware/font_ex.h (1)
font_init(34-34)legacy/firmware/font_ex.c (1)
font_init(101-111)
🪛 Ruff (0.11.9)
legacy/script/i18n.py
48-48: Missing return type annotation for public function write_i18n_header
Add return type annotation: None
(ANN201)
48-48: Missing type annotation for function argument languages_map
(ANN001)
48-48: Unused function argument: languages_map
(ARG001)
48-48: Missing type annotation for function argument items_count
(ANN001)
89-89: Missing return type annotation for public function write_i18n_source
Add return type annotation: None
(ANN201)
89-89: Missing type annotation for function argument languages_map
(ANN001)
89-89: Unused function argument: languages_map
(ARG001)
🪛 GitHub Actions: Prebuild checks
legacy/script/i18n.py
[warning] 62-62: Flake8: W293 blank line contains whitespace
[warning] 66-66: Flake8: W293 blank line contains whitespace
[warning] 74-74: Flake8: W293 blank line contains whitespace
[warning] 77-77: Flake8: W293 blank line contains whitespace
[warning] 84-84: Flake8: W293 blank line contains whitespace
[warning] 112-112: Flake8: W293 blank line contains whitespace
[warning] 116-116: Flake8: W293 blank line contains whitespace
[warning] 122-122: Flake8: W293 blank line contains whitespace
[warning] 126-126: Flake8: W293 blank line contains whitespace
[warning] 131-131: Flake8: W293 blank line contains whitespace
[warning] 135-135: Flake8: W293 blank line contains whitespace
[warning] 140-140: Flake8: W293 blank line contains whitespace
[warning] 143-143: Flake8: W293 blank line contains whitespace
[warning] 149-149: Flake8: W293 blank line contains whitespace
🔇 Additional comments (26)
legacy/firmware/i18n/locales/zh_tw.inc (1)
145-145: LGTM! Improved UI text clarity.The change from a question to a confirmation statement makes the interface more direct and user-friendly. This aligns well with the broader i18n standardization effort.
legacy/firmware/i18n/locales/es.inc (1)
145-145: LGTM! Clearer user interface text.Changing from "¿Usar este passphrase?" to "Confirmar Passphrase" makes the action more direct and clear. Great improvement for user experience consistency.
legacy/firmware/i18n/keys.h (1)
312-312: LGTM! Comment updated to match UI text.Good practice to keep macro comments in sync with the actual localized strings. This maintains consistency with the passphrase prompt changes across all language files.
legacy/firmware/menu_core.c (1)
26-32:Details
✅ Verification successful
Validate loop boundary update
The loop now usesI18N_ITEMS_COUNT. Ensure thelanguages_en[]array length matches this constant.
🏁 Script executed:
#!/bin/bash # Count entries in the English locale entries=$(grep -c '^\s*"' legacy/firmware/i18n/locales/en.inc) # Extract the defined item count count=$(grep -R "#define I18N_ITEMS_COUNT" legacy/firmware/i18n/i18n.h | awk '{print $3}') echo "entries=$entries, I18N_ITEMS_COUNT=$count"Length of output: 243
Loop boundary is correct
We counted 383 entries inlocales/en.incandI18N_ITEMS_COUNTis 383. The loop boundary matches the array length—no change needed.legacy/firmware/trezor.c (1)
291-293: Ensure font init happens before UI render
Movingfont_init()ahead oflayoutHome()andusbInit()loads fonts early. Good fix.legacy/firmware/i18n/locales/ja.inc (1)
145-145: Update Japanese passphrase prompt
Changed to “パスフレーズを確認” to match the new confirmation style.legacy/firmware/i18n/locales/pt_br.inc (1)
145-145: Update PT-BR passphrase prompt
Replaced question with “Confirmar Passphrase” for consistency.legacy/firmware/i18n/locales/en.inc (1)
145-145: Standardize English passphrase text
Switched from a question to “Confirm Passphrase”. Clearer intent.legacy/firmware/i18n/locales/ko_kr.inc (1)
145-145: LGTM! UI text improvement.The change from question format to confirmation format aligns with the broader standardization effort across all language files. This improves UI consistency.
legacy/firmware/i18n/locales/de.inc (1)
145-145: LGTM! Consistent with other language updates.The German translation correctly changes from question format to confirmation format, matching the standardization pattern applied across all locale files.
legacy/firmware/config.c (2)
286-286: Good refactoring to use constant.Replacing
langs_lenwithI18N_LANGUAGE_ITEMSimproves consistency and aligns with the i18n system refactoring. The constant-based approach is more maintainable.
397-397: Consistent with refactoring pattern.Same improvement as the previous loop - using the constant instead of variable provides better consistency across the codebase.
legacy/firmware/config_emu.c (2)
693-693: LGTM! Consistent with config.c changes.The constant-based approach matches the refactoring in the main config file and maintains consistency between emulator and hardware implementations.
934-934: Good consistency across implementations.This change maintains the same pattern as config.c, ensuring both emulator and hardware configs use the standardized constant for language iteration.
legacy/firmware/i18n/locales/zh_cn.inc (2)
145-145: LGTM! Standardized passphrase prompt.The change from "使用此 Passphrase？" to "确认 Passphrase" aligns with UI standardization across languages. Good consistency improvement.
261-261: Good fix for character display issue.Replaced incorrect vertical bar character "｜" with proper Chinese character "键". This likely fixes display problems on certain devices.
legacy/firmware/gettext.c (1)
14-17: Excellent refactoring! Simplified language lookup.Replaced complex switch-case with clean table lookup. Bounds checking with
I18N_LANGUAGE_ITEMSprevents array overflows. Much more maintainable.legacy/firmware/i18n/i18n.c (2)
5-25: Great readability improvement!Multi-line array initialization is much clearer than compact format. Easy to read and maintain.
36-45: Smart centralization of language data.The
languages_tablearray provides unified access to all language strings. Clean design that simplifies thegettextXfunction.legacy/firmware/i18n/i18n.h (3)
8-9: Good use of fixed constants.Replacing variable counts with fixed constants makes the system more predictable and easier to reason about.
11-20: Excellent enum design.Explicit integer values ensure correct indexing into
languages_table. Type safety bonus for language identifiers.
34-34: Proper external declaration.Matches the
languages_tabledefinition ini18n.c. Enables centralized language access across the codebase.legacy/firmware/layout2.c (4)
469-476: LGTM - Language count standardization looks good.The replacement of
langs_lenwithI18N_LANGUAGE_ITEMSconstant makes the code more maintainable and aligns with the standardization mentioned in the summary.
485-485: Consistent usage of the new constant.Good - this maintains consistency with the other changes in the function.
3640-3687: Language enum migration looks solid, including Korean support.The switch from numeric indices to enum constants (
I18N_LANG_EN,I18N_LANG_ZH_CN, etc.) improves code readability. The addition ofI18N_LANG_KO_KRcase addresses the Korean display bug mentioned in the PR title. Bitmap positioning adjustments appear appropriate for different language text layouts.
2392-2392:Details
❓ Verification inconclusive
Verify text rendering simplification doesn't break layout.
The function now draws text centered at a fixed position instead of handling multi-line splitting. Ensure this change doesn't cause text overflow or truncation issues.
Let me check if there are other usages of this function to understand the impact:
🏁 Script executed:
#!/bin/bash # Description: Find usages of layoutTxConfirmPage to verify the text rendering change impact # Expected: Should show where this function is called and what kind of text is passed rg -A 3 -B 3 "layoutTxConfirmPage"Length of output: 6323
Verify centered confirm-text won’t overflow
The new
layoutTxConfirmPageuses a singleoledDrawStringCenterAdapterat a fixed Y-position (21px). Make sure none of the strings passed ever exceed the display width or require wrapping. Review these call sites:
- legacy/firmware/filecoin.c
- legacy/firmware/polkadot.c
- legacy/firmware/cosmos.c
- legacy/firmware/algorand.c
- legacy/firmware/ada.c
- legacy/firmware/alephium/alph_layout.c
- legacy/firmware/tron.c
- legacy/firmware/ton_layout.c
- legacy/firmware/layout2.c
If any
datacan be longer than the screen, either revert to multi-line handling (e.g.layoutPaginated) or add explicit breaks.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (12)
legacy/firmware/fsm_msg_nostr.h(1 hunks)legacy/firmware/i18n/keys.h(1 hunks)legacy/firmware/i18n/locales/de.inc(1 hunks)legacy/firmware/i18n/locales/en.inc(1 hunks)legacy/firmware/i18n/locales/es.inc(1 hunks)legacy/firmware/i18n/locales/ja.inc(1 hunks)legacy/firmware/i18n/locales/ko_kr.inc(1 hunks)legacy/firmware/i18n/locales/pt_br.inc(1 hunks)legacy/firmware/i18n/locales/zh_cn.inc(2 hunks)legacy/firmware/i18n/locales/zh_tw.inc(1 hunks)legacy/firmware/layout2.c(4 hunks)legacy/firmware/trezor.c(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
legacy/firmware/trezor.c (2)
legacy/firmware/font_ex.c (1)
font_init(101-111)legacy/firmware/font_ex.h (1)
font_init(34-34)
legacy/firmware/fsm_msg_nostr.h (2)
legacy/util.c (1)
bracket_replace(103-126)legacy/util.h (1)
bracket_replace(81-81)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Gen check
- GitHub Check: Style check
- GitHub Check: Defs check
🔇 Additional comments (12)
legacy/firmware/i18n/locales/zh_tw.inc (1)
145-145: Consistent translation update. Traditional Chinese string now reads "確認 Passphrase", matching other locales.legacy/firmware/i18n/locales/es.inc (1)
145-145: Spanish prompt updated correctly. Matches other locales’ "Confirm Passphrase".legacy/firmware/trezor.c (1)
291-294: Initialize fonts before layout. Movingfont_init()ahead ofusbInit()ensures fonts are ready before UI rendering, aligning with updated i18n support.legacy/firmware/i18n/locales/pt_br.inc (1)
145-145: Consistent Brazilian Portuguese update. Prompt now reads "Confirmar Passphrase", matching other locales.legacy/firmware/i18n/locales/ja.inc (1)
145-145: LGTM! UI text standardization looks good.The change from question format to confirmation statement improves UI consistency. Japanese translation uses appropriate katakana for the technical term.
legacy/firmware/i18n/locales/en.inc (1)
145-145: Excellent UI text improvement!The change from "Use This Passphrase?" to "Confirm Passphrase" is clearer and more actionable. Sets good foundation for other language translations.
legacy/firmware/i18n/locales/de.inc (1)
145-145: German translation looks solid!"Passphrase bestätigen" follows good localization practice by keeping the technical term in English. Consistent with the UI standardization effort.
legacy/firmware/i18n/locales/ko_kr.inc (1)
145-145: Perfect fix for the Korean display bug!The change from long question format to concise "Passphrase 확인" likely resolves UI layout issues. Shorter text fits better in constrained UI space while maintaining clarity.
legacy/firmware/i18n/locales/zh_cn.inc (2)
145-145: Approve standardized passphrase terminology.Good change! The shift from "使用此 Passphrase？" to "确认 Passphrase" aligns with the standardization across all language files.
261-261: Approve typo correction in PIN instruction.Correct fix! Replacing "｜" with "键" makes the PIN input instruction accurate and readable.
legacy/firmware/layout2.c (2)
485-488: Confirm boundary with constant
You replaced the oldlangs_lencheck withI18N_LANGUAGE_ITEMS - 1. Double-check thatI18N_LANGUAGE_ITEMStruly matches the size ofi18n_langs[]to avoid off-by-one issues.
3640-3690:Details
❓ Verification inconclusive
Consolidate per-language icon coordinates
Each case inlayoutInputDirectionnow hardcodes different bitmap coordinates. This duplication risks drift and makes adjustments hard. Consider centralizing these values in a small lookup table keyed byi18n_lang_t, then drive theoledDrawBitmapcalls from that table. Also, manually test each locale to confirm the arrows render correctly.
Consolidate per-language icon coordinates
The switch inlayoutInputDirectionhardcodes X/Y values for each language. This duplicates logic and makes future tweaks error-prone. I recommend:
- Define a small lookup table (e.g. struct array) keyed by
i18n_lang_tholding up/down bitmap coordinates and any direction offsets- In
layoutInputDirection, read from that table instead of inlining values- Call
oledDrawBitmapusing the table entries- Manually test every locale to verify correct arrow placement
This centralizes your data, cuts down on drift, and simplifies adjustments.
3 participants
Summary by CodeRabbit