-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Extract i18n string into i18n/_defaults (base of translations) #4514
Conversation
js/scripts/build-i18n.js
Outdated
|
||
import i18nstrings from '../.build/i18n/i18n/en.json'; | ||
|
||
const FILE_HEADER = '// Copyright 2015-2017 Parity Technologies (UK) Ltd.\n// This file is part of Parity.\n\n// Parity is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n\n// Parity is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n\n// You should have received a copy of the GNU General Public License\n// along with Parity. If not, see <http://www.gnu.org/licenses/>.\n\n'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use template string here, would be more readable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually did have it that way around, will put it back. (Don't expect to edit it at all, but still more readable.)
js/scripts/build-i18n.js
Outdated
import fs from 'fs'; | ||
import path from 'path'; | ||
|
||
import i18nstrings from '../.build/i18n/i18n/en.json'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is babel-node
used only for these import
s ? If it is, could use require I think...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const/let is supported, so only imports. I would prefer keeping the import just for code-base consistency. (Although there is a slight compiler indirection involved here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really mind but it adds quite some work for not so much. All the other scripts are using standard Node syntax
I would have something like that : const fs = require('fs');
const path = require('path');
const i18nstrings = require('../.build/i18n/i18n/en.json');
const FILE_HEADER = `// Copyright 2015-2017 Parity Technologies (UK) Ltd.
// This file is part of Parity.
// Parity is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// Parity is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// You should have received a copy of the GNU General Public License
// along with Parity. If not, see <http://www.gnu.org/licenses/>.\n\n`;
const I18NPATH = path.join(__dirname, '../src/i18n/_default');
main();
function getSections () {
const sections = {};
// create an object map of the actual inputs
Object.keys(i18nstrings).forEach((fullKey) => {
const defaultMessage = i18nstrings[fullKey].defaultMessage;
const keys = fullKey.split('.');
let outputs = sections;
keys.forEach((key, index) => {
if (index === keys.length - 1) {
outputs[key] = defaultMessage;
} else {
if (!outputs[key]) {
outputs[key] = {};
}
outputs = outputs[key];
}
});
});
return sections;
}
function main () {
console.log('\n-------------------------------------------------------');
console.log('------------------ START: i18n exports ----------------');
console.log('-------------------------------------------------------');
const sections = getSections();
// create the index.js file
const sectionKeys = Object.keys(sections).sort();
const i18Exports = sectionKeys
.map((key) => {
return `export ${key} from './${key}.json';`;
})
.join('\n');
const indexPath = path.join(I18NPATH, 'index.js');
console.log(' --> writing exports to', indexPath);
fs.writeFileSync(indexPath, `${FILE_HEADER}${i18Exports}\n`, 'utf8');
console.log('\n --> writing', sectionKeys.length, 'exported message files in', I18NPATH);
// create the individual section files
sectionKeys.forEach((key) => {
const sectionText = JSON.stringify(sections[key], null, 2);
fs.writeFileSync(path.join(I18NPATH, `${key}.json`), `${sectionText}`, 'utf8');
});
console.log('\n------------------------------------------------------');
console.log('------------------ DONE: i18n exports ----------------');
console.log('------------------------------------------------------\n');
} |
This exports all strings as JSON, and the |
As discussed on chat - Just want to make the outputs the same as what people are to produce - in-place editing. Currently (not cast in stone, obviously), we import exactly the same formats. It could be -
Actually, this format is better than json for editing, however probably not the best overall. So +1 for ease, but not perfect. So open to suggestions to make editing as easy as possible. Current format is better than json (which is the standard), but def. not the best. |
Strangely enough, your |
@jacogr in the generated _default/settings.js many settings.xx.xx.labels are now missing, while they were there before. |
Not sure why JSON is less prone to editing than a JS Object. The script would be safer IMO (use a simple |
JSON vs JS - No lint checking, no duplicate key checking, no option for commenting. My preference would actually be for something where standard tools can be used, so preferably neither. (However the current is def better than standard JSON) |
@h3ll0fr13nd Good catch. It only pulls in the strings that are actually in the source, not the special cases (where settings in one since it gets duplicated in a number of places). Will address and pull them in alongside. |
js/scripts/build-i18n.js
Outdated
|
||
// export a section as a flatenned string (non-JSON, rather JS export) | ||
function createExportString (section, indent) { | ||
if (Object.prototype.toString.call(section) === '[object String]') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (typeof section === 'string')
Well, lint checking won't add more than the key duplicate check. For the comments, the file is generated automatically so I don't really see where comments would be added ? |
Ahh, yes, these files are generated automatically. However, they are the base of what translators work on and give back. These files are not means for reading by machines, taking them, reading, translating - the result of those will be e.g. this PR by our first external translation - https://github.com/ethcore/parity/pull/4484/files#diff-b4da45a3972c2df6a228f6eed0856c04R17) ^^^ That is what we need to make easy, JSON is not it :) |
I'm torn between both arguments. JSON is certainly easier to edit for non-JS contributors. Also it is closer to other static i18n formats. JS has the advantage of allowing e.g. comments. Formats like json5 unfortunately require further tooling. |
@jacogr So do you think that JS Objects are more human-readable than JSON ? Not sure why that would be. Plus For comments, we could add a description to the |
100%, much more readable, no question, especially with nesting instead of chained keys. (And as I stated before, I would actually prefer something else - but still need some investigation here to see what standard tools properly support. In this case it should be for use by translation tools, not humans. Most translators won't have techy chops.) The JS as it stands will give a start to the volunteers which has technical know-how, e.g. NL & ES. (Not the end-game) |
Missed this before. This is a very strong argument for my to stick with plain JSON. |
Just to be sure, what I am suggesting is having this : {
"ui": {
"account": { ... }
}
} instead of this : {
ui: {
account: { ... }
}
} |
That doesn't match what is actually exported by the build plugins :) We still go through and mangle it into submission to get that format. Your example is exactly what I did have originally, however much easier to work with that type of structure in JS as opposed to JSON, just because of
|
@h3ll0fr13nd It now pulls in the missing non-standard location strings as well. It should match up with what you have been working with thus far. |
js/scripts/build-i18n.js
Outdated
@@ -46,8 +48,17 @@ const SRCPATH = path.join(__dirname, '../.build/i18n/i18n/en.json'); | |||
outputIndex(sectionNames); | |||
})(); | |||
|
|||
// helper to merge in section into defaults | |||
function deepMerge (object, source) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_.defaultsDeep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooooh! Cool. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, that works for me ! We might now need a script that would ease translation, like copying the files to the different languages, having warning for deprecated keys (or automatically removing them if we feel confident), have some sort of status on translation...
+1 I was thinking about that as well. (Also a reason why I believe the next step is really standard tool support, just need a gap to start looking - don't want to write too much ourselves that are already handled elsewhere. However, for the people who will be editing/copying the defaults, it needs to be easy, so checking will be in order.) |
All
i8n/_default/*
files generated by thescripts/build-i18n.js
script.