Openrefine: Use best practices in i18n

[remram44]

I applaud your attempt at internationalizing OpenRefine, however you did not read the manual. As you know other languages have their own grammar and using the English grammar on all languages is obviously not going to work:

https://github.com/OpenRefine/OpenRefine/blob/e5fdf48680a6f810c84706671344dc93d9a4fb75/main/webapp/modules/core/scripts/dialogs/clustering-dialog.js#L245

The jquery.i18n library has a lot of awesome features inherited from gettext such as string interpolation (and even custom plurals). This is not the only location in the code where messages are made up from pieces using concatenation, please use interpolation instead so that languages that use a different word order don't look broken.

Other example:

https://github.com/OpenRefine/OpenRefine/blob/e5fdf48680a6f810c84706671344dc93d9a4fb75/main/webapp/modules/core/scripts/preferences.js#L86

Please just use a construct like:

if (window.confirm($.i18n('confirm-delete-key', key))) { 

It also seems that you added sections in your translation JSON, placing JSON objects where messages should be, which is probably going to break all the machinery (and is responsible for the awkward i18n(...)[...]). Please read library's manuals or at least their repository's README before proceeding.

[wetneb]

@remram44 I completely agree! Pull requests are more than welcome.

[remram44]

Fixing this would mean all translation files have to be adapted (though I suspect the program is not very usable in non-English locales anyway), so a commitment from the project leads is necessary before work can happen here.

[wetneb]

@remram44 The project leads (if there is such a thing) do not master all the languages that we currently support, so we cannot fix these translations ourselves… So I don't really see what you mean! What "commitment from project leads" do you expect? The people who wrote the code that you quoted are not active in OpenRefine anymore.
Feel free to invalidate existing translations, people will go to Weblate to fix them if they use the translation.

[remram44]

Feel free to invalidate existing translations, people will go to Weblate to fix them if they use the translation.

Sounds good! I just don't want to do work to then hear that "that would break translations" and have the patch rejected. But if that's acceptable then it's fine.

I will see if I can do that but I'm short on time at the moment, so if somebody else wants to grab this go ahead.

[remram44]

It doesn't look like you are using jquery.i18n (from wikipedia) but recurser/jquery-i18n. Commit adding it is 817c6cc8d4468c5bf8e776cbe2f384385e74bf69, which doesn't mention the source.

Furthermore, although the whole source is reproduced, no attribution is done to Dave Perrett, the author; this violates the terms of the MIT license (quite a feat...)

cc @Blakko

[ostephens]

@remram44 thanks for noting the missing licence - now submitted a PR to add this in and correct this oversight

[jackyq2015]

@remram44 Thanks for the suggestion. Just one question, do you see any drawback of the form $.i18n._('core-index')["delete-key"] except the complexity

[remram44]

It is a bit long to type, but the issue here is that this returns a single string, and you get no string interpolation or custom plural machinery, necessary for proper i18n.

For example this is problematic:

https://github.com/OpenRefine/OpenRefine/blob/e5fdf48680a6f810c84706671344dc93d9a4fb75/main/webapp/modules/core/scripts/dialogs/clustering-dialog.js#L246

Using jquery.i18n, gettext or another state-of-the-art i18n library, you do $.i18n('core-clusters-filtered', clusters.length, this._clusters.length) or similar instead of concatenations, allowing the translator to see the full sentence in one go, reorder the inserted values if appropriate, and properly pluralize words in the translation.