Betaflight-configurator: RFC: i18n work

[chmelevskij]

Problem

There are 2 main approaches on how to deal with i18n.
chrome app way
I18next

With Vue work there was some workarounds to get around the formatting as well.

Solution

Current Vue implementation used vue-i18n package, because it was easiest to add for initial development. I18next lists few vue plugins which might be used would be good to investigate those or implement custom formatter for vue-i18n which would work with the current messages.

TBD

[McGiverGim]

To me any of the two solutions is good. Now what we have is i18next but with a wrapper (similar to the custom formatter method) here:

https://github.com/betaflight/betaflight-configurator/blob/59076c0c3aba5e83e0ae47edebb5f0780215298f/src/js/localization.js#L62-L83

We use i18next interpolation but we use nesting too https://www.i18next.com/translation-function/nesting (I don't know if the Vue vue-i18n let's do it too). And it has other several benefits that we don't use like plurals, formatting, etc. but can be useful in a future.

[mikeller]

Switching to vue-i18n sounds like a reasonable approach for once we have axed Chrome web app support.
As to whether we should be changing the translations format or by using a wrapper will be down to if we can change the format without too much of an impact on our volunteer translators.

[McGiverGim]

We can change the format in all languages and upload the files to Crowdin, but it needs to be a synchronized work.

[mikeller]

The 'synchronised' part is where I think it will become problematic - trying coordinate with all of our translators will be hard.

[McGiverGim]

No, we don't need to synchronize with them. When I talk about synchronization I'm talking about modify and upload all in the less time possible.
We need to modify all the messages files at the same time in local, and upload all of them to Crowdin. We need to do this fast to not break any translator work.
I think is easier to add, at least at first the wrapper or the custom formatter.

[mikeller]

We need to modify all the messages files at the same time in local, and upload all of them to Crowdin. We need to do this fast to not break any translator work.

That's what I mean - we need to synchronise with translators to get them to finish whatever they are working on so that we have the data at rest, or else we will be overwriting their work.

[chmelevskij]

So dropping chrome way of doing i18n would be the first thing then? Are there any PRs/issues related to that?

[McGiverGim]

@chmelevskij I think @mikeller is simply talking about dropping the Chrome support that is here: https://github.com/betaflight/betaflight-configurator/pull/2186

I think the best and clean way, is to support the Custom Formatting as you proposed: https://kazupon.github.io/vue-i18n/guide/formatting.html#custom-formatting

The plugins for i18next are ok too, I suppose i18next is most powerful, but we need to add to them some custom formatter too (it does not accept the old Chrome format neither) and I don't know if this is possible (this is the reason why we have the old wrapper). If you find a way to do it maintaining all the benefits of Vue to me is ok.

What is clear is that since now, we must only accept new PR that use objects with named formatting for the translations messages. Both solutions support this and the result string is more clear than with positional parameters and helps the job of translators. I only use this kind of formatting since we support this.

[McGiverGim]

I forgot to mention that the custom formatting need to support the i18next nesting too that we use, but it seems to be the same format than the Vue translations so maybe it is supported out of the box. We will need to test it.

[McGiverGim]

I have started to try to "fix" the i18n ugly hack, but I have noticed that it fixed some discrepance in our strings that I did not notice in the PR...

https://github.com/betaflight/betaflight-configurator/blob/7fcc9efc72af9723fd4f1f053b55bcea6e78ef88/locales/en/messages.json#L627-L644

We have the status bar with different kind of strings: some of them include the parameter, others not and the view adds the value next to the string.
Now we have a component, ReadingStat.vue for it. The hack simply removed the $1% at the end in all strings and adds the number later in different span.
What we want is a general solution, this can work (in an ugly way) for this cases, but not for others. Maybe some translation needs a different unit, or different order, so we need to change it.

@mikeller what must we do here?

1. Use two different components. I don't see it.
2. Modify the messages.json file to unify all of them. This will introduce work for translators if we do this for other components.
3. Make the component "smart", maybe doing just the opposite of what we are doing, if we don't have the $1 in the message, add it at the end. Is more general. But this can happen for other elements.

The correct is 2, but if we don't want to modify translations, I think the only solution is 3.

Some other idea? We need to decide what to do in cases like this before we try to use a custom formatter.

@chmelevskij if we decide to go with 3, what is the best way to add this, with a filter like you did? making a computed parameter for the message and simply use it always?

[McGiverGim]

Maybe another solution:

1. Reduce the granularity or the components. Have only an StatusBar.vue component that includes all of the status bar. In this way is not as clean and reusable as the actual approach, but is a start...

[chmelevskij]

I don't think that having a single StatusBar.vue is a good idea, at least not when it's already been implemented as separate components. Would have to redo it again in the future.

If there is more than 1 format to specify values in translated strings now it would be the best time to unify it. If not it will always be a hack, wrapper or some other thing.

As I've mentioned the tranlsation package was mainly chosen because of module support and my initial PR didn't have any bundler. But since I've introduced rollup we could use https://github.com/panter/vue-i18next which would understand i118next strings already like https://github.com/betaflight/betaflight-configurator/blob/7fcc9efc72af9723fd4f1f053b55bcea6e78ef88/locales/de/messages.json#L23

As intermediate solution could get the wrapper and shove it in https://www.i18next.com/translation-function/formatting#basic

I hope this makes sense

[McGiverGim]

Yes, I have looked at the vue-i18next and I was thinking to test it... we use too nesting: https://www.i18next.com/translation-function/nesting so this will fix two of the problems not having to "implement" it too.

But when trying to do some tests I found the discrepancy problem of different strings in the POC. I think we can fix it changing them in this case and let this in a correct manner, maybe is a good idea to change the old strings each time we move to Vue the affected part. In this way the work for translators will be very few (it can took years to migrate the complete app).

I don't think we can use the rollup to do the conversion of messages.json to i18next format. Is not only to change the $1, $2 to {{1}} {{2}} or similar, we need to change the way we pass the parameters to it (i18next uses objects and in the old way we pass a list of parameters, alone or in array form). I wanted to do some tests, but I prefer to fix first the problem.

If the solution is to fix the messages file, we only have 62 strings with the old translation format of $n, and maybe some others that do not have the parameters need to be added, but will be very few in a migration of several years as I said.

@mikeller what do you think about:

• Use the i18next plugin for Vue.
• When something is migrated, if the string in in the old format, migrate it to the new. It will be work for the translators, but we will end with better code, better messages file, and there will be only few phrases that Crowdin can help to translate using the translation memory. And they are not too much.

This seems a good plan to me.

[chmelevskij]

I don't think we can use the rollup to do the conversion of messages.json to i18next format

Rollup had nothing to do with these 😉 It's just that i18next-vue didn't work without rollup in place. With rollup you can import it and use it as a package.

[chmelevskij]

Also I'm not familiar with the translation process, but can't same person translate multiple languages? Of course some of them will need proper translation, but taken the status bar messages as an example, all of the have exactly same format and the only thing which needs changing is the value, which is the same anyways on all of them. Literally every language just had $1% so I can't see why it's impossible to change it to {{value}}% in one go

[McGiverGim]

Nobody says it's impossible, but we use an external platform (https://crowdin.com/project/betaflight-configurator), that limits our option. We don't modify directly in source files in github, so we need to modify the English an all languages and upload them one by one. If some translator is working since we make the change, we can replace his work, for example.
Is more secure to modify it step by step. Modifying them when needed by the Vue components and letting the translator translate it again. As I said this will be a very long process so the overload for the translator will be minimum.

[mikeller]

@McGiverGim:

Use the i18next plugin for Vue.
When something is migrated, if the string in in the old format, migrate it to the new. It will be work for the translators, but we will end with better code, better messages file, and there will be only few phrases that Crowdin can help to translate using the translation memory. And they are not too much.

I am with you on this - this seems to be the right approach, and we'll have to absorb this extra effort as the 'cost of migrating to a new framework'.
'Making the components smart' might work for this case, but in a lot of other cases of variable substitution it will not, as positioning of elements inside natural sentences varies with language.
One thing that is quite handy in our current approach is the capability to directly substitute translation keys within other translation keys - this is often used to reference tab names in help texts, or to build warnings that contain a generic problem descripton and a contextualised mitigation. Is there any facility to do this in vue-i18n?

@chmelevskij:

I don't think that having a single StatusBar.vue is a good idea, at least not when it's already been implemented as separate components. Would have to redo it again in the future.

I don't think that 'it is already done this way' is a valid justification to not redo something if it is found to have been done in the wrong way. And when it comes to deciding what the right granularity is for componentisation then I think this one is indeed disputable - after all the status bar is just presented as a single string.

[McGiverGim]

Perfect then. It seems a plan. I've a PR ready to remove the need of the .message at all the translations in Vue, but need to merge the change of backend first.
@chmelevskij van you finish the i18next plugin migration to go with this approach?

[McGiverGim]

@chmelevskij better wait until I finish with this change in the backend, I think I have an idea to integrate all in a clean way...

[McGiverGim]

I have opened a PR with the final solution to all this problems, I think is the correct direction to fix all of this.

[McGiverGim]

@chmelevskij I have observed some errors related with the i18n, that appears in the console:

All works, but I suspect is trying to get some translation before our i18n system is loaded. When loaded it replaces the unknown messages (I suppose the reactivity of the variables), so all works, but is a bug that is good to fix.

Do you think I'm right? What is the solution for this? Maybe load the translations in the init.js code before loading the components?

[chmelevskij]

Is it on master? vue-i18n was replaced with vue-i18next so it should go away once you merge your changes in.

[McGiverGim]

No, it fails with both approaches.

But I have found a workaround. As I expected, it was because the async nature of all our heritage of html processes. I have changed the i18next backend by a filesystem sync one, and the error is gone. This is one of the benefits of removing the Chrome version, we can make use of this new features.

I will update the PR of migration with this new approach in some minutes, doing some tests now...

[McGiverGim]

Fixed here: https://github.com/betaflight/betaflight-configurator/pull/2245