search

openhab / openhab-vscode

VS Code extension for openHAB configuration files
https://marketplace.visualstudio.com/items?itemName=openhab.openhab
Eclipse Public License 2.0
159 stars 47 forks source link

Initial i18n implementation #89

Closed Confectrician closed 6 years ago

Confectrician commented 6 years ago

Introducing i18n support

This pull request continues #56. Goal is a basic implementation of the nls feature to support localitzation of the vscode openHAB extension.

General Information

This PR is already available here in an early state, to have a place for converation/discussion directly.

How the localization technically works

Since i had to look up many things in even more different repositories and since there was already some confusion in the first review, i thought that it would be useful to have some basic kind of overview.

General

Here is an example of how the german localization files currently are organized: image

As you can see, we have to follow the tree structure of the extension code directly.

Source language

The source language (english in our case) is defined

Package loca keys

Are implemented with percentage marks %openhab.searchDocs.title%

localize() function keys

Are implemented as first paramter of the function. Second parameter is the corresponding source language string. localize('init.noActiveEditor.text', 'No editor is active')

Note: Every file that should have localization entries must import the following module:

import * as nls from 'vscode-nls'
const localize = nls.loadMessageBundle()
Using dynamic values is possible with

localize('channelTemplate.stateError.text', '"{0}" is a {1} channel.', channel.uid, channel.kind)

Testing translations

This is only possible by packaging a .vsix file through gulp build. Further information will be posted here by edit in the next days.

Currently Known Problems

Credits

Also-by: Kuba Wolanin hi@kubawolanin.com Signed-off-by: Jerome Luckenbach github@luckenba.ch

Confectrician commented 6 years ago

Ok, i think i got a huge progress in making at least the english source language already localizable. Tests are outstanding.

If someone has nothign to do, feel free to check if i have missed any strings, that are shown somewhere to the user and have to be translated.

Confectrician commented 6 years ago

And here some information about the avaiable languages: image

Sorry @kubawolanin. No polish availabe for vscode at the moment. (And some others too.) This means that we will not be able to translate the extension at the moment, or test it. It seems that the locale is choosen for everything from this config.

We can prepare that, but it may not be shown until code supports those languages too. With that knowledge i would also suggest to restrict possible crowdin translations to the available languages. (cc @ThomDietrich here)

Maybe we should push Microsofts transifex vscode repo a bit forward. 😄

General testing update

German seems to work from what i haveseen already. I will try to get some error modals to test that too and maybe add some french translations just for testing.

Confectrician commented 6 years ago

Since there was already confusion about some changes in the first review, i have added a quick version of the "how loca in vscode works" section in the initial posting..

It is not complete, but should give everyone a first hint on how it works and why there is this or that dependency now in the package.json.

Confectrician commented 6 years ago

Small update: Testing went ok for the package related contents. The files are located correct and through packaging the "real" loca files get generated.

"Real" Files get generated for the -in code- loca too, but they are not used at the moment or i can't get them to be used while testing. I have tried several things and oriented the structure completely to the example, but can't get it to use the generated german files. Maybe i will open up an issue in the nls repo. maybe someone's able to find the mistake i have made.

ThomDietrich commented 6 years ago

Hello @Confectrician ! Great work! I have just one question from a quick browse through. You state right in the beginning:

i18n folder ... subfolders ... ISO 639 ... countries

Now ISO 639 primarily addresses languages, not countries or regions. Can you influence the way those folders are named? Many systems including crowdin work with the language[-REGION] format. See:

My main question is: Will we be able to add a "Mexican Spanish" translation in parallel to a generic "Spanish" translation?

Confectrician commented 6 years ago

Hey @ThomDietrich,

The PR-Overview is also work in progress right now. My bad. Folders are created according to ISO 639-3 which also supports sublanguages/dialects. https://en.wikipedia.org/wiki/List_of_ISO_639-3_codes

For example deu (main german) and gct (german, cologne dialect) would be supported. I have not looked up spanish and mexican spanish now, but i think this should be possible later.

Please keep in mind, that i was not able to set up other languages than the one vscode itself is translated into, right now. If we want to support more language independent from vscode environment variables, we may have to introduce an own setting for this at a later point.

Also i was not able to get the -in code- translations working until now and i have (to be honest) no idea what went wrong. But basically the translation on package level is ready for production from my view. (cc @kubawolanin here)

The -in code_ translations will simply fall back to the english version now.

Confectrician commented 6 years ago

Some testing update from my side:

ThomDietrich commented 6 years ago

Oh that's too bad! Good luck! Thanks for your engagement with this new thask ;)

@kubawolanin @Confectrician I had a talk with Kai and he convinced me to do some last needed things in the whole "openHAB Localization Project". If everything works out as expected, I'll add the openhab2-addons repository as yet another crowdin project and post an Announcement and Invite (for users to translate and become proofreader) in the openHAB community. This is supposed to happen this weekend.

Do you see any chance to push this PR forward before the weekend? For me it would be enough to have the translations strings files merged, even without being used currently, so that I can create, set up and announce the respective crowdin project. Wdyt?

kubawolanin commented 6 years ago

@ThomDietrich @Confectrician I believe we could go ahead and merge the strings files as soon as the code changes don't break the functionality. Merging it is just a half of the success. I need to release a new version to the Marketplace afterwards. So if we're confident everything works, we can merge it. @Confectrician how do you feel about the idea?

Cheers

Confectrician commented 6 years ago

Hi,

The current situation is:

The only disadvantage is that the in code transalations, simply fall back to english. So there's not really a breaking behaviour existing currently.

I haven't heard anything about my issue yet. My proposal would be, to revert the in code Loca (just to keep thigns clean) for now and merge the working package level translation. This is a huge basic effort anyway already.

We can then add more loca files/strings afterwards.

@ThomDietrich Whats the workflow, if new files have to added to crowdin? We can't prevent making new files here, since the file structure has to reflect the current code always.

@kubawolanin would you agree with starting only with the package level translations for now?

BR Jerome

kubawolanin commented 6 years ago

@kubawolanin would you agree with starting only with the package level translations for now?

Absolutely! Thank you so much for pushing it forward! :clinking_glasses:

ThomDietrich commented 6 years ago

@ThomDietrich Whats the workflow, if new files have to added to crowdin?

@Confectrician the important detail is: will your ongoing efforts to eventually also support in-code translation break with the strings file design as it is right now. I guess the answer is no, that's all we need to care for currently. If at a later point you realize that a second set of localization files needs to be added, those can then easily be included in crowdin. If those in-code translations can be added to the existing translation files, this would even be easier.

We can't prevent making new files here, since the file structure has to reflect the current code always.

I'm not sure what you mean. As soon as we started to import strings in crowdin, every change - i.e. addition, modification or deletion of individual strings - will result in translation and approval needs for those. Hence the worst thing we could do right now is merge a set of english strings/files which will at a later point be replaced by files at other locations or in a different syntax. I believe this is the crucial detail to figure out now, before the merge.

Confectrician commented 6 years ago

the important detail is: will your ongoing efforts to eventually also support in-code translation break with the strings file design as it is right now. I guess the answer is no

Your guess is correct, see below for explanation.

I'm not sure what you mean. As soon as we ...

Simple example: If we add another TypeScript File (For this example: myNewFeature.ts) for a new function or something equivalent in the extension source code, we have to add a corresponding myNewFeature.i18n.json file under the corresponding folders.

Let me refer to https://github.com/Microsoft/vscode-node-debug2 as a showcase of an existing i18n configuration.

Package Level: image

Source Level (Extension Source on the right for comparison): image

The folder structure i18n/LANGUAGE_Code/... will be the consistent, but might grow/expand a bit if we add functionality. That's the only thing i wanted to clarify here.

I have already searched for other extension projects, that use crowdin, but it seems only vscode icons does that and it seems that they have implemented their own loca management. Unfortunately nothing i am able to learn from.

Confectrician commented 6 years ago

OK revert and cleanup done. Pls do a check @kubawolanin

ThomDietrich commented 6 years ago

I see. Just to clarify:

  1. You (that is the developers of openhab-vscode) will provide files like package.i18n.json including the base english strings.
  2. These files will then be imported by crowdin
  3. Translated files are sourced back by crowdin in a PR to e.g. i18n/deu/package.i18n.json.

New files next to package.i18n.json are not an issue. For me it would be best if I could generalize the import filter to something like "Import all *.i18n.json files from folder xyz."

@kubawolanin I'm not sure if I asked this before: As those files are json files we will bump into that tricky behavior of crowdin where they will export empty strings again. Will they be an issue? @Confectrician if you made the following change in one of the localized files, what is presented in UI?

"openhab.basicUI.title": "",
Confectrician commented 6 years ago

New files next to package.i18n.json are not an issue. For me it would be best if I could generalize the import filter to something like "Import all *.i18n.json files from folder xyz."

Unfortunately package.i18n.json is an edge case here. When we use the localize function as intended (see first post for a basic explanation), the source language strings are stored in the .ts files directly (within the localize function). Of course this is more than worse for importing them in crowdin. So we need to provide a file based solution.

My suggestion would be using i18n/eng/ as a master for everything. This way we would have exactly the same file structure, as needed for the localized languages later.

So again in simple words: We maintain the contents of i18n/eng/ as an import master for crowdin and take care of updating it. Crowdin maintainers only use that folder for import and don't care about how we fill it.

We will then find a solution (gulpfile is already there, yeah) to maintain source language files automatically on a later point.

@Confectrician if you made the following change in one of the localized files, what is presented in UI?

I have tested that a while ago already with an empty fra file. (Every loca key had an empty string.) Translations where generated in source languages in that case. But i can't fully confirm that for a "partly localized" file. I will test it later and give feedback.

Confectrician commented 6 years ago

Bad News @ThomDietrich:

Of only one string is empty, it will stay empty after packaging. Seems that it is not checked for every string on its own.

Confectrician commented 6 years ago

Just for documentation:

Unfortunately still nothing new from Microsoft/vscode-nls#16

Confectrician commented 6 years ago

Hey @ThomDietrich and @kaikreuzer,

FYI: Since it seems impossible to get an answer from vscode devs/contributors in a nearer time, you should go one for the loca announcement without vscode included.

I was already thinking of simply builing an own loca implementation. It seems that there are some extension out there that have done it this way.

Confectrician commented 6 years ago

I was already thinking of simply builing an own loca implementation. It seems that there are some extension out there that have done it this way.

Own implementation it will be at some point of time. Seems vscode contributors are not able to answer questions and i am not willed to invest further time in poking or waiting.