Multiple language support #107
Conversation
| - Add the new translation file and language tag to `./languages.js` | ||
| - If needed: make overrides in `languages/overrides/{language_tag}.json` | ||
| - Create an empty translation file `echo "{}" > languages/translations/{language_code}.json` | ||
| - Add the new translation file and language code to `./languages.js` |
There was a problem hiding this comment.
the new translation file
This can be removed, since the code for loading each language file is not present anymore. It's only an array of language codes now that needs to be edited.
test/languages_test.js
Outdated
| var translations = languages.get(['en', 'fr']); | ||
|
|
||
| assert.deepEqual(Object.keys(translations).sort(), ['fr', 'en'].sort(), | ||
| 'only returns en and fr'); |
There was a problem hiding this comment.
This line break is a bit weird. I'd prefer either having everything on one line, but since that may not be possible for length reasons I'd prefer one-line-per-parameter:
assert.deepEqual(
Object.keys(translations).sort(),
['fr', 'en'].sort(),
'only returns en and fr'
);
Readme.md
Outdated
| `options.hooks.tokenizedInstruction` | optional | `function(instruction)` | A function to change the raw instruction string before tokens are replaced. Useful to inject custom markup for tokens | ||
| `options.languages` | optional | `en` `de` `zh-Hans` `fr` `nl` `ru` | Array of language identifiers that should be supported. Default is loading all language files, which can be huge on websites |
There was a problem hiding this comment.
We don’t want developers to get the impression that only these six languages are supported, given that we currently support ten (with more to come). Perhaps add “etc.” and link to this folder for the full list of supported languages?
There was a problem hiding this comment.
That's a good idea. I'm changing this.
index.js
Outdated
| options.hooks.tokenizedInstruction = ((_options || {}).hooks || {}).tokenizedInstruction; | ||
| options.languages = (_options || {}).languages || languages.supportedCodes; | ||
|
|
||
| // TODO Validate language |
There was a problem hiding this comment.
Out of curiosity, what sort of validations do you have in mind?
|
This PR looks like it’ll address #106. |
|
I tried to integrate these changes into osrm-frontend, but that failed. The reason is browserify, which we use to bundle the I'm good with actually walking back on the requirement of #106 to select which language files to load, given it makes bundling of js files so hard for browsers. For the server-side, this isn't actually a big problem, since the memory required is marginal. We should definitely keep switching languages during compile though. |
patjouk
changed the title
patjouk
force-pushed
the
selectively-languages
branch
from
ced083e
to
b4cdbf5
Compare
This PR introduces support for multiple language support.
Right now all language files are always loaded. With this PR, the developer can decide which language files to require. During test instruction compilation, the language used can be chosen. This allows us to scale this project to large numbers of languages, without bloating data downloaded in browsers for all users.The module API as well as internal APIs have breaking changes, since we had to change several function signatures.
TODOs
CC @freenerd
edit: We will continue to load every language for now because browserify doesn't support dynamic requires. (cf @freenerd's comment)