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

Support per-layout translations entries #4495

Merged
merged 4 commits into from
Jan 22, 2025
Merged

Support per-layout translations entries #4495

merged 4 commits into from
Jan 22, 2025

Conversation

romanz
Copy link
Contributor

@romanz romanz commented Jan 13, 2025

Following #4489.

Also, use layout names for translation font keys.

@romanz romanz self-assigned this Jan 13, 2025
Copy link

github-actions bot commented Jan 13, 2025

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@romanz romanz requested a review from obrusvit January 13, 2025 12:42
@romanz
Copy link
Contributor Author

romanz commented Jan 14, 2025

I will reorganize the commits above, in order to separate the text-modifying edits into a separate commit (which can be dropped - to avoid UI changes).

@romanz romanz force-pushed the romanz/translations branch from 8952a98 to 4148e87 Compare January 14, 2025 17:32
@romanz
Copy link
Contributor Author

romanz commented Jan 14, 2025

i.e., there is no diff between 8952a98 and 4148e87, just reorged the commits' content by the above force-push.

@romanz romanz marked this pull request as ready for review January 14, 2025 17:39
@romanz romanz changed the title WIP: Support per-layout translations entries Support per-layout translations entries Jan 14, 2025
@romanz romanz linked an issue Jan 14, 2025 that may be closed by this pull request
8 tasks
@romanz romanz added the translations Put this label on a PR to run tests in all languages label Jan 15, 2025
@romanz romanz force-pushed the romanz/translations branch from 27b8ec8 to 8cdeca7 Compare January 20, 2025 11:50
@romanz romanz linked an issue Jan 21, 2025 that may be closed by this pull request
8 tasks
Copy link
Contributor

@obrusvit obrusvit left a comment

Choose a reason for hiding this comment

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

Thank you. It works as expected, we can now specify different strings per layout and per language. There's one caveat though:

  • if we misspell a layout name in en.json, then everything is OK because the mistake is caught in compile time 👍
  • if we misspell a layout name in other lang, e.g. cs.json then it fallbacks to english version 🤔 I wonder if we can do something about it now.

Note that this problem is not new because if we miss the whole string in cs.json then we also get fallback to english version without any warning. I'd probably fix this issue separatedly.

python/src/trezorlib/_internal/translations.py Outdated Show resolved Hide resolved
It would simplify `TranslationsDir.load_lang` method.

[no changelog]
Done by:
```
cd core/translations
python ./cli.py merge ??.json
```

[no changelog]
@romanz romanz force-pushed the romanz/translations branch from c5353ae to dee67ff Compare January 22, 2025 07:47
@romanz
Copy link
Contributor Author

romanz commented Jan 22, 2025

  • if we misspell a layout name in other lang, e.g. cs.json then it fallbacks to english version 🤔 I wonder if we can do something about it now.

Good point, thanks!
Maybe we can add a sanity check to be sure that only "allowed" layout names are used as keys for translations?
For example, we can collect all layout names used in cs.json "translations" section and make sure they correspond to a valid layout, using the "fonts" section keys:

image

@romanz romanz merged commit 2061199 into main Jan 22, 2025
140 checks passed
@romanz romanz deleted the romanz/translations branch January 22, 2025 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
translations Put this label on a PR to run tests in all languages
Projects
Status: Approved
Development

Successfully merging this pull request may close these issues.

Enable Crowdin workflow for firmware translations
2 participants