[knx] Improve localization

[holgerfriedrich]

- introduce localization of error messages
- add new strings for common exceptions
- provide helper functions for translation
- add test cases

Signed-off-by: Holger Friedrich mail@holger-friedrich.de

[holgerfriedrich]

This is still work in progress. It adds German translations to the KNX binding.

I would appreciate any feedback on the general approach how to add the translations.

1. property files: German translations are based on latest Crowdin export. Is it a valid approach to copy the export to get a start for a new language?
2. property files: I have added a few lines which are not part of the offical binding yet. If I got it correctly, I should get this into knx.propeties. Once it is merged on master, it will show up on Crowdin and can be translated. When will the automatic PR process start for a language? What is the trigger for the automatic PR process? Who approves the changes on Crowdin?
3. property files: Will the automatic PR process wipe out all unknown lines?
4. property files: Any hints regarding namespace for additional strings/error messages?
5. LocaleProvider.getlocale() -> will it always return the same locale or does this change dynamically?

Thanks!

[holgerfriedrich]

@lolodomo could you give some input to the questions above or recommend whom to ask? Thanks!

[jlaur]

I would appreciate any feedback on the general approach how to add the translations.

This page might also shed some light: https://www.openhab.org/docs/developer/utils/i18n.html#managing-translations

1. property files: German translations are based on latest Crowdin export. Is it a valid approach to copy the export to get a start for a new language?

I don't understand this question, can you elaborate?

2. property files: I have added a few lines which are not part of the offical binding yet. If I got it correctly, I should get this into knx.propeties. Once it is merged on master, it will show up on Crowdin and can be translated.

Yes.

When will the automatic PR process start for a language?

At least once per day.

What is the trigger for the automatic PR process?

I believe it's a time trigger, but it can also be triggered manually by admins.

Who approves the changes on Crowdin?

Preferably proof-readers, but I'm not sure we have many yet. So actually, this question I cannot answer - @lolodomo, can you?

3. property files: Will the automatic PR process wipe out all unknown lines?

Yes.

4. property files: Any hints regarding namespace for additional strings/error messages?

Each binding has its own namespace, so you don't need to use any prefixes to secure uniqueness. Best advice for custom texts would be to try to avoid potential conflicts with framework-generated strings/prefixes.

5. LocaleProvider.getlocale() -> will it always return the same locale or does this change dynamically?

It will change dynamically when user changes regional settings.

[holgerfriedrich]

@jlaur thanks for the feedback.
I think I will add a test and then set this PR to ready for review the next days.

I would appreciate any feedback on the general approach how to add the translations.

1. property files: German translations are based on latest Crowdin export. Is it a valid approach to copy the export to get a start for a new language?

I don't understand this question, can you elaborate?

This was about how to introduce a new language to a binding. Is it intended to download the property file for the new language from Crowdin and add it to the binding - and after this the bot takes over for further updates? Or should I just add empty files like knx_de.properties and the bot will take over as well?

Who approves the changes on Crowdin?

Preferably proof-readers, but I'm not sure we have many yet. So actually, this question I cannot answer - @lolodomo, can you?

That would be interesting. We seem to have unapproved translations for KNX (de). According to the docs the bot will not include them, correct?

[jlaur]

1. property files: German translations are based on latest Crowdin export. Is it a valid approach to copy the export to get a start for a new language?

I don't understand this question, can you elaborate?

This was about how to introduce a new language to a binding. Is it intended to download the property file for the new language from Crowdin and add it to the binding - and after this the bot takes over for further updates? Or should I just add empty files like knx_de.properties and the bot will take over as well?

Neither. New languages are automatically provided from Crowdin.

Who approves the changes on Crowdin?

Preferably proof-readers, but I'm not sure we have many yet. So actually, this question I cannot answer - @lolodomo, can you?

That would be interesting. We seem to have unapproved translations for KNX (de). According to the docs the bot will not include them, correct?

Correct.

[holgerfriedrich]

This was about how to introduce a new language to a binding. Is it intended to download the property file for the new language from Crowdin and add it to the binding - and after this the bot takes over for further updates? Or should I just add empty files like knx_de.properties and the bot will take over as well?

Neither. New languages are automatically provided from Crowdin.

@jlaur For me it is still not clear what actually triggers a new language file to be added in the first place. For example, there is a knx.properties, but no file knx_de.properties in resources/OH-INF/i18n yet. Though there are translations available at Crowdin, the bot does not create a PR. I suppose this is because knx_de.properties does not exist.

[jlaur]

For me it is still not clear what actually triggers a new language file to be added in the first place. For example, there is a knx.properties, but no file knx_de.properties in resources/OH-INF/i18n yet. Though there are translations available at Crowdin, the bot does not create a PR. I suppose this is because knx_de.properties does not exist.

The important sentence from the documentation is this one: "Please note that all texts must be translated and approved in order to be included." So for a language to be included in PR's created from Crowdin, 100% of strings must be translated, and 100% must be approved. So it is not because knx_de.properties does not exist, but because at least one of those criteria are not fulfilled.

In German only 82% are approved. @cweitkamp, would you be able to have a look at the pending knx translations and/or possible shed some more light on the approval process?

[cweitkamp]

In German only 82% are approved. @cweitkamp, would you be able to have a look at the pending knx translations and/or possible shed some more light on the approval process?

Done.

[jlaur]

@holgerfriedrich - the German translation is now merged: #13448.

[holgerfriedrich]

thanks, @cweitkamp !
@jlaur I added test cases and removed knx_de.properties from the commit as it is now already merged. I will introduce new strings via CrowdIn once this PR is merged. I took over you mock classes from hdpowerview.

[holgerfriedrich]

@kaikreuzer could you please have a look? I'd Like to close this before getting deeper into the data secure topic again... Thanks!

[jlaur]

Thanks! I've added some feedback. Perhaps @kaikreuzer would like to have a second look.

[holgerfriedrich]

@jlaur thanks for the review! I think I addressed all of your comments. I left one conversation open, explaining why I used the enum to implement the singleton.

[kaikreuzer]

@kaikreuzer could you please have a look?

@jlaur is the much better reviewer for this, so he can merge right away once he is done.

[jlaur]

I've added some minor comments. Let's merge after you had a chance to have a look. :-)

[holgerfriedrich]

I've added some minor comments. Let's merge after you had a chance to have a look. :-)

@jlaur, thanks. Finally, you convinced me to exchange the strings for the unit test :-)

[jlaur]

LGTM, thanks!

[holgerfriedrich]

@cweitkamp maybe you could approve the translations for the new strings introduced in this PR once again? I have uploaded German translations to Crowdin. Thanks!

[cweitkamp]

@cweitkamp maybe you could approve the translations for the new strings introduced in this PR once again?

Done.