Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translations management CLI #955

Merged
merged 22 commits into from
Nov 27, 2018
Merged

Translations management CLI #955

merged 22 commits into from
Nov 27, 2018

Conversation

lyyder
Copy link
Contributor

@lyyder lyyder commented Nov 19, 2018

translations_cli

Adds a command line tool that can be used to manage translations:

  • see what keys are missing compared to English translations
  • add a missing translation

Testing

To try the out the tool, copy src/translations/en.json to src/translations/es.json for example and remove a few lines from the es translations file.

@lyyder
Copy link
Contributor Author

lyyder commented Nov 19, 2018

Let's see at what point we get the default languages translation files added to the project. Not the tool expects to have es, de, and fr translations in place. Maybe all the language definitions could be removed and added as the corresponding files are added. That way this could be merged now without waiting for the translations to finish.

translations Outdated
sourceLang = SOURCE_LANG.value;
targetLang = answers.lang;
source = require(filePath(sourceLang));
target = require(filePath(targetLang));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably better to not use require to future-proof this since "Require is synchronous function and is called only once, which means the calls receive a cached result. If the file is updated you can't re-read it using this method"

https://stackabuse.com/reading-and-writing-json-files-with-node-js/

@Gnito
Copy link
Contributor

Gnito commented Nov 20, 2018

@lyyder how about reading current files from the folder?

$ cat asdf.js 
const testFolder = './src/translations/';
const fs = require('fs');

fs.readdirSync(testFolder).forEach(file => {
  console.log(file);
})
$ node asdf.js 
en.json
es.json

Then it would use your TARGET_LANGS for printing names (es -> Spanish) but it would default to language code extracted from file name (found file fi-FI.json -> no known name -> ´fi-FI´)

This way we don't need to keep track any specific translations and customizers could simply delete unnecessary files.

Copy link
Contributor

@Gnito Gnito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!
You probably should ask from Thomas if the usage is clear and if color codings work.
And it might be good to check that our translators know when to use this (i.e. this is more beneficial to our customizers when they are updating from upstream - Thomas et. al could use WTI if they prefer that).

@lyyder
Copy link
Contributor Author

lyyder commented Nov 27, 2018

@Gnito I've split the execution based on comments form @thomasmalbaux so that after translating a key the app returns to the missing keys list instead of languages list. This works better when syncing one translations file with the source lang, which is the case with our customizers.

An executable provides little or nothing extra in this case and running
the tool as 'node translations.js' is more universal across platforms
than './translations'.
Use 'y' and 'N' instead of filtering them to booleans as the filtered
value is printed to the console and is confusing is differs from the one
user typed.
Split execution so that after translating a key the application does not
return to the beginning but to the keys listing with the same language.
@lyyder lyyder merged commit e2aca7a into master Nov 27, 2018
@lyyder lyyder deleted the translations-cli branch November 27, 2018 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants