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

Add translations for keyboard layouts. #3251

Merged
merged 8 commits into from
Jan 8, 2024

Conversation

demiankatz
Copy link
Member

@demiankatz demiankatz commented Dec 7, 2023

@ThoWagen, I know that we had previously discussed aliasing keyboard layouts to language codes to accomplish translations, but when I started investigating the possibility, I discovered some significant semantic differences. While there is a lot of overlap, in some cases the layouts correspond to region names rather than language names, or there are multiple layouts for the same language. Thus, I think we'll have better control if we give the layouts their own text domain. This PR adds the text domain, adjusts the template to use it, and fixes an invalid layout name in one of the configuration examples.

Translations are not 100% complete -- we might be able to fill in a few more as further progress is made on #3200 -- but I think this is at least a strong starting point.

TODO

@demiankatz demiankatz requested a review from ThoWagen December 7, 2023 13:45
@demiankatz demiankatz added this to the 10.0 milestone Dec 7, 2023
@ThoWagen
Copy link
Contributor

ThoWagen commented Dec 8, 2023

Would it be possible to allow aliasing into other text domains so that we could add an alias.ini in KeyboardLayout and use it for the layouts that are language names? Then we could add e.g. german = ISO639-3::deu. But maybe that would make things more complicated rather than easier. I don't think that your approach in #3249 supports aliasing into other text domains and it would probably not be straight forward to implement. So this is just an idea.

@ThoWagen ThoWagen mentioned this pull request Dec 8, 2023
6 tasks
@demiankatz
Copy link
Member Author

@ThoWagen, I'll address the idea of aliasing into other TextDomains on #3249 shortly, since I think that's probably the best place to have that conversation.

However, if we decide that it is not practical to directly alias across TextDomains, we could still accomplish the same effect here by making these language files inherit from the equivalent ISO639-3 TextDomains and then defining the aliases locally. This would use more cache space/memory because it would duplicate all the translations, but it would allow us to reduce language file maintenance.

I'll leave this open until we resolve this issue one way or the other.

@demiankatz
Copy link
Member Author

@ThoWagen, I have reimplemented this using the new aliasing mechanism, and it seems to be working correctly. I believe it is now ready for your review.

Copy link
Contributor

@ThoWagen ThoWagen left a comment

Choose a reason for hiding this comment

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

This looks good to me :)

Only one small hint: I'm not sure if it is intended or not that "farsi" is aliased as "ISO639-3::per" but also translated as "Farsi" in en.ini.

@demiankatz
Copy link
Member Author

Thanks, @ThoWagen! Regarding your observation, it was intentional (I thought it might be better to keep the translation closer to the code when choosing between two synonyms for the language), but in retrospect, I don't think it's necessary, and it might actually be more confusing, so I'll remove the override and just keep the alias.

@demiankatz demiankatz merged commit 1d83403 into vufind-org:dev Jan 8, 2024
7 checks passed
@demiankatz demiankatz deleted the keyboard-layout-translations branch January 8, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants