Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Side by Side Diff: csv-export.js

Issue 29636585: Issue 6171 - create CSV exporter and importer for translations (Closed)
Patch Set: Addressed most of the comments Created May 4, 2018, 10:38 a.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | package.json » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
(Empty)
1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH
4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation.
8 *
9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 * GNU General Public License for more details.
13 *
14 * You should have received a copy of the GNU General Public License
15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
16 */
17
18 /* globals require, process */
Thomas Greiner 2018/05/07 15:16:33 Detail: No need to list `require` as a global anym
saroyanm 2018/05/16 17:06:34 Done.
19
20 "use strict";
21
22 const fs = require("fs");
23 const path = require("path");
24 const csv = require("csv");
25 const {promisify} = require("util");
26 const execFile = promisify(require("child_process").execFile);
27 const csvParser = promisify(csv.parse);
28 const readDir = promisify(fs.readdir);
Thomas Greiner 2018/05/07 15:16:34 This variable is unused.
saroyanm 2018/05/16 17:06:32 Done.
29 const readFile = promisify(fs.readFile);
Thomas Greiner 2018/05/07 15:16:37 This variable is unused so let's use it instead of
saroyanm 2018/05/16 17:06:33 The variable is used. see, line: 131
Thomas Greiner 2018/05/22 17:22:48 Still, you're using `fs.readFile()` in `readJson()
saroyanm 2018/06/05 15:03:45 Done.
30 const glob = promisify(require("glob").glob);
31 const readJsonPromised = promisify(readJson);
Thomas Greiner 2018/05/07 15:16:36 Why do you call `promisify()` on your own function
saroyanm 2018/05/15 18:59:48 Because of current comment -> https://codereview.a
Thomas Greiner 2018/05/22 17:22:47 That's because we cannot change how native Node fu
32
33 const localesDir = "locale";
34 const defaultLocale = "en_US";
35
36 let headers = ["filename", "StringID", "Description", "Placeholders",
Thomas Greiner 2018/05/07 15:16:38 Detail: Unlike the other headers, "filename" doesn
saroyanm 2018/05/16 17:06:33 Done.
37 defaultLocale];
38 let outputFileName = "translations-{repo}-{hash}.csv";
39
40 /**
41 * Export existing translation - files into CSV file
42 * @param {string[]} [filesFilter] - fileNames filter, if omitted all files
43 * will be exported
44 */
45 function exportTranslations(filesFilter)
46 {
47 let mercurialCommands = [];
48 // Get Hash
49 mercurialCommands.push(execFile("hg", ["id", "-i"]));
50 // Get repo path
51 mercurialCommands.push(execFile("hg", ["paths", "default"]));
52 Promise.all(mercurialCommands).then((outputs) =>
53 {
54 // Remove line endings and "+" sign from the end of the hash
55 let [hash, filePath] = outputs.map((item) =>
56 item.stdout.replace(/\+\n|\n$/, ""));
Thomas Greiner 2018/05/07 15:16:34 Suggestion: You're calling the array "outputs" but
saroyanm 2018/05/16 17:06:31 Done.
57 // Update name of the file to be output
58 outputFileName = outputFileName.replace("{hash}", hash);
59 outputFileName = outputFileName.replace("{repo}", path.basename(filePath));
60
61 // Read all available locales and default files
62 return glob(`${localesDir}/**/*.json`, {});
Thomas Greiner 2018/05/07 15:16:37 Detail: The second argument is optional according
saroyanm 2018/05/16 17:06:32 Done.
63 }).then((filePaths) =>
64 {
65 // Reading all existing translations files
66 return Promise.all(filePaths.map((filePath) => readJsonPromised(filePath)));
67 }).then(csvFromJsonFileObjects);
68 }
69
70 /**
71 * Creating Matrix which reflects output CSV file
72 * @param {Array} fileObjects - array of file objects created by readJson
Thomas Greiner 2018/05/07 15:16:37 Detail: Arrays of objects are described as `Object
saroyanm 2018/05/16 17:06:34 Done.
73 */
74 function csvFromJsonFileObjects(fileObjects)
75 {
76 let locales = [];
77 // Create Object tree from the Objects array, for easier search
78 // ex.: {dektop-options.json: {en_US: {...}, {de: {...}, {ru: {...}}}
79 let dataTreeObj = fileObjects.reduce((accumulator, fileObject) =>
Thomas Greiner 2018/05/07 15:16:38 This `reduce()` is no longer just populating a sin
saroyanm 2018/05/16 17:06:32 Done.
Thomas Greiner 2018/05/22 17:22:48 You replaced the `reduce()` call with a for-of loo
saroyanm 2018/06/04 15:17:59 I don't understand this comment, what is wrong wit
Thomas Greiner 2018/06/04 16:58:21 The problem I'm referring to is that you're handli
saroyanm 2018/06/05 10:35:26 Got it, thanks for the clarification, I'll fix tha
saroyanm 2018/06/05 15:03:44 Done.
80 {
81 if (!fileObject)
82 return accumulator;
83
84 let {fileName, locale} = fileObject;
85 if (!locales.includes(locale))
86 locales.push(locale);
87
88 if (!accumulator[fileName])
89 {
90 accumulator[fileName] = {};
91 }
92 accumulator[fileName][locale] = fileObject.strings;
93 return accumulator;
94 }, {});
95
96 let fileNames = Object.keys(dataTreeObj);
97 if (filesFilter.length)
98 fileNames = fileNames.filter((item) => filesFilter.includes(item));
99
100 locales = locales.filter((locale) => locale != defaultLocale).sort();
Thomas Greiner 2018/05/07 15:16:34 Why do you sort the locales? That step didn't exis
saroyanm 2018/05/16 17:06:33 I think I did that to have consistent CSV file gen
Thomas Greiner 2018/05/22 17:22:49 Acknowledged.
101 // Create two dimensional strings array that reflects CSV structure
102 let csvArray = [headers.concat(locales)];
103 for (let fileName of fileNames)
104 {
105 let strings = dataTreeObj[fileName][defaultLocale];
106 for (let stringID of Object.keys(strings))
107 {
108 let fileObj = dataTreeObj[fileName];
109 let {description, message, placeholders} = strings[stringID];
110 let row = [fileName, stringID, description || "",
111 JSON.stringify(placeholders), message];
112
113 for (let locale of locales)
114 {
115 let localeFileObj = fileObj[locale];
116 let isTranslated = !!(localeFileObj && localeFileObj[stringID]);
117 row.push(isTranslated ? localeFileObj[stringID].message : "");
118 }
119 csvArray.push(row);
120 }
121 }
122 arrayToCsv(csvArray);
123 }
124
125 /**
126 * Import strings from the CSV file
127 * @param {string} filePath - CSV file path to import from
128 */
129 function importTranslations(filePath)
130 {
131 readFile(filePath, "utf8").then((fileObjects) =>
132 {
133 return csvParser(fileObjects);
134 }).then((dataMatrix) =>
135 {
136 let headLocales = dataMatrix.shift().slice(4);
137 let dataTreeObj = {};
138 for (let rowId in dataMatrix)
139 {
140 let row = dataMatrix[rowId];
141 let [currentFilename, stringId, description, placeholder, ...messages] =
142 row;
143 if (!stringId)
144 continue;
145
146 stringId = stringId.trim();
147 // Check if it's the filename row
148 if (!dataTreeObj[currentFilename])
149 dataTreeObj[currentFilename] = {};
150
151 description = description.trim();
152 for (let i = 0; i < headLocales.length; i++)
153 {
154 let locale = headLocales[i].trim();
155 let message = messages[i].trim();
156 if (!message)
157 continue;
158
159 // Create Object tree from the Objects array, for easier search
160 // ex.: {dektop-options.json: {en_US: {...}, {de: {...}, {ru: {...}}}
161 if (!dataTreeObj[currentFilename][locale])
162 dataTreeObj[currentFilename][locale] = {};
163
164 let localeObj = dataTreeObj[currentFilename][locale];
165 localeObj[stringId] = {};
166 let stringObj = localeObj[stringId];
167
168 // We keep string descriptions only in default locale files
169 if (locale == defaultLocale)
170 stringObj.description = description;
171
172 stringObj.message = message;
173 if (placeholder)
174 stringObj.placeholders = JSON.parse(placeholder);
175 }
176 }
177 writeJson(dataTreeObj);
178 });
179 }
180
181 /**
182 * Write locale files according to dataTreeObj
183 * @param {Object} dataTreeObj - ex.:
184 * {dektop-options.json: {en_US: {...}, {de: {...}, {ru: {...}}}
185 */
186 function writeJson(dataTreeObj)
187 {
188 for (let fileName in dataTreeObj)
189 {
190 for (let locale in dataTreeObj[fileName])
191 {
192 let filePath = path.join(localesDir, locale, fileName);
193 let orderedJSON = orderJSON(dataTreeObj[fileName][locale]);
Thomas Greiner 2018/05/07 15:16:37 Suggestion: Usually, we'd use the word "sort" inst
saroyanm 2018/05/16 17:06:31 Done.
Thomas Greiner 2018/05/22 17:22:47 Note that while you renamed the variable, the func
saroyanm 2018/06/05 15:03:44 Done.
194 let fileString = JSON.stringify(orderedJSON, null, 2);
195
196 // Newline at end of file to match Coding Style
197 if (locale == defaultLocale)
198 fileString += "\n";
199 fs.writeFile(filePath, fileString, "utf8", (err) =>
200 {
201 if (err)
202 {
203 console.error(err);
204 }
205 else
206 {
207 console.log(`Updated: ${filePath}`);
208 }
209 });
210 }
211 }
212 }
213
214 /**
215 * This function currently rely on nodeJS to sort the object by keys
Thomas Greiner 2018/05/07 15:16:35 Detail: Technically, it relies on V8's implementat
Thomas Greiner 2018/05/07 15:16:36 Typo: Replace "rely" with "relies".
saroyanm 2018/05/16 17:06:34 Done.
saroyanm 2018/05/16 17:06:35 Done
Thomas Greiner 2018/05/22 17:22:48 Acknowledged.
216 * @param {Object} unordered - json object
217 * @returns {Object}
218 */
219 function orderJSON(unordered)
220 {
221 const ordered = {};
222 for (let key of Object.keys(unordered).sort())
223 {
224 ordered[key] = unordered[key];
225 if (unordered[key].placeholders)
226 ordered[key].placeholders = orderJSON(unordered[key].placeholders);
227
228 ordered[key] = unordered[key];
229 }
230 return ordered;
231 }
232
233 /**
234 * Convert two dimensional array to the CSV file
235 * @param {Array} csvArray - array to convert from
236 */
237 function arrayToCsv(csvArray)
238 {
239 csv.stringify(csvArray, (err, output) =>
240 {
241 fs.writeFile(outputFileName, output, "utf8", (error) =>
242 {
243 if (!error)
244 console.log(`${outputFileName} is created`);
245 else
246 console.error(error);
247 });
248 });
249 }
250
251 /**
252 * Reads JSON file and assign filename and locale to it
253 * @param {string} filePath - ex.: "locales/en_US/desktop-options.json"
254 * @param {function} callback - fileName, locale and strings of locale file
255 * Parameters:
256 * * Error message
257 * * Object containing fileName, locale and strings
258 */
259 function readJson(filePath, callback)
260 {
261 let {dir, base} = path.parse(filePath);
262 fs.readFile(filePath, "utf8", (err, data) =>
263 {
264 if (err)
265 {
266 callback(err);
267 }
268 else
269 {
270 callback(null, {fileName: base, locale: dir.split("/").pop(),
Thomas Greiner 2018/05/07 15:16:34 The path delimiter is OS-specific so please use `p
Thomas Greiner 2018/05/07 15:16:35 Detail: Let's avoid doing multiple things in a sin
saroyanm 2018/05/16 17:06:31 Done.
saroyanm 2018/05/16 17:06:32 Done. I think you meant `path.sep`
Thomas Greiner 2018/05/22 17:22:47 Yep, you're right.
271 strings: JSON.parse(data)});
272 }
273 });
274 }
275
276 /**
277 * Exit process and log error message
278 * @param {String} error error message
279 */
280 function exitProcess(error)
281 {
282 console.error(error);
283 process.exit();
Thomas Greiner 2018/05/07 15:16:35 We should pass a number that's not `0` when exitin
saroyanm 2018/05/16 17:06:31 OOps, thanks, done.
284 }
285
286 // CLI
287 let helpText = `
288 About: Converts locale files between CSV and JSON formats
289 Usage: csv-export.js [option] [argument]
290 Options:
291 -f [FILENAME] Name of the files to be exported ex.: -f firstRun.json
292 option can be used multiple times.
293 If omitted all files are being exported
294
295 -o [FILENAME] Output filename ex.:
296 -f firstRun.json -o {hash}-firstRun.csv
297 Placeholders:
298 {hash} - Mercurial current revision hash
299 {repo} - Name of the "Default" repository
300 If omitted the output fileName is set to
301 translations-{repo}-{hash}.csv
302
303 -i [FILENAME] Import file path ex: -i issue-reporter.csv
304 `;
305
306 let argv = process.argv.slice(2);
307 let stopExportScript = false;
308 // Filter to be used export to the fileNames inside
309 let filesFilter = [];
310
311 for (let i = 0; i < argv.length; i++)
312 {
313 switch (argv[i])
314 {
315 case "-h":
316 console.log(helpText);
317 stopExportScript = true;
Thomas Greiner 2018/05/07 15:16:38 Suggestion: What about moving the argument process
saroyanm 2018/05/16 17:06:35 As was mentioned here -> https://codereview.adbloc
Thomas Greiner 2018/05/22 17:22:48 Ok.
318 break;
319 case "-f":
320 // check if argument following option is specified
Thomas Greiner 2018/05/07 15:16:37 Detail: The code seems self-explanatory so maybe b
saroyanm 2018/05/16 17:06:35 Done.
321 if (!argv[i + 1])
322 {
323 exitProcess("Please specify the input filename");
324 }
325 else
Thomas Greiner 2018/05/07 15:16:34 Detail: This else-statement is redundant because w
saroyanm 2018/05/16 17:06:34 Done.
326 {
327 filesFilter.push(argv[i + 1]);
328 }
329 break;
330 case "-o":
331 if (!argv[i + 1])
332 {
333 exitProcess("Please specify the output filename");
334 }
335 else
Thomas Greiner 2018/05/07 15:16:35 Detail: This else-statement is redundant because w
saroyanm 2018/05/16 17:06:33 Done.
336 {
337 outputFileName = argv[i + 1];
338 }
339 break;
340 case "-i":
341 if (!argv[i + 1])
342 {
343 exitProcess("Please specify the import file");
344 }
345 else
Thomas Greiner 2018/05/07 15:16:35 Detail: This else-statement is redundant because w
saroyanm 2018/05/16 17:06:33 Done.
346 {
347 let importFile = argv[i + 1];
348 importTranslations(importFile);
349 stopExportScript = true;
350 }
351 break;
352 }
353 }
354
355 if (!stopExportScript)
356 exportTranslations(filesFilter);
OLDNEW
« no previous file with comments | « no previous file | package.json » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld