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

ICU message format for translations #2045

Merged
merged 16 commits into from
May 29, 2024
Merged

Conversation

flatron4eg
Copy link
Contributor

@flatron4eg flatron4eg commented Mar 26, 2024


Added support for MessageFormat from ICU for all the translation lines.


@marticliment
Copy link
Owner

marticliment commented Mar 26, 2024

Could you please explain further what ICU does? Thanks!

@flatron4eg
Copy link
Contributor Author

flatron4eg commented Mar 27, 2024

Here in description of library that I used (see Supported formats) some good examples of formats which can be used. Most used case for russian language for example would be plurals, cause in english you have only two cases - 1 item is without "s" ending and the others - with "s", which you are handling right now directly in the code with "if's" (but with ICU support you can just use line f.e. {0} {0,plural,one {update}, other {updates}} available). In russian (also almost all the slavic languages) there are more cases with different endings, f.e. "яблоко" (an apple) with different numerals before would be "1 яблоко", "2 яблока", "5 яблок" and so on. Also, you can format dates, numbers in different locales, currencies and more.

@flatron4eg
Copy link
Contributor Author

One more note - Tolgee already handles ICU format: it has highlighting and it shows readable result when translation is saved

@marticliment
Copy link
Owner

Makes sense. I will test the PR and let you know.

The translation files don't need to be modified?

@flatron4eg
Copy link
Contributor Author

No, we can just use new functions when translating the lines in Tolgee, old ones will work as before. But there can be one issue - if you support all the old versions and they can fetch new translation lines, if they will use ICU - old versions of WingetUI would not handle them properly, not sure what is the best way to resolve this.

@marticliment
Copy link
Owner

marticliment commented May 14, 2024

Hello @flatron4eg, I have been doing testing and I am afraid this PR would break older versions of WingetUI. Therefore, I can't merge it.

@flatron4eg
Copy link
Contributor Author

flatron4eg commented May 14, 2024

@marticliment But since you already changed path to language files with UniGet rebranding (https://github.com/marticliment/WingetUI/blob/527a35a77bd0f86dc820dd33acd09c8dea55a00a/src/UniGetUI/Core/Data/LanguageData.cs#L101), older versions will not get any updates from github, cause there are no files on that old path. IMO this is the best moment to make this change.

@marticliment
Copy link
Owner

That is true, I did not think about that...

@marticliment marticliment reopened this May 14, 2024
@flatron4eg
Copy link
Contributor Author

Resolved all the conflicts, is there anything else have to be done before merge?

@marticliment
Copy link
Owner

No, everything should be fine. I will review changes and merge it as soon as I have a moment to do some testing

@marticliment
Copy link
Owner

Before merging this PR I will merge #2161. Then I will fix the conflicts and do the merge

@flatron4eg
Copy link
Contributor Author

I know you are busy so I resolved the conflicts and fixed codacy issues. There is one failing test but it seems unrelated

@marticliment
Copy link
Owner

Thanks❤️

I will check the tests and merge

@marticliment marticliment merged commit 3744dee into marticliment:main May 29, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants