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

Unified 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.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | package.json » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: csv-export.js
===================================================================
new file mode 100644
--- /dev/null
+++ b/csv-export.js
@@ -0,0 +1,356 @@
+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-present eyeo GmbH
+ *
+ * Adblock Plus is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Adblock Plus 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 Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/* 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.
+
+"use strict";
+
+const fs = require("fs");
+const path = require("path");
+const csv = require("csv");
+const {promisify} = require("util");
+const execFile = promisify(require("child_process").execFile);
+const csvParser = promisify(csv.parse);
+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.
+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.
+const glob = promisify(require("glob").glob);
+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
+
+const localesDir = "locale";
+const defaultLocale = "en_US";
+
+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.
+ defaultLocale];
+let outputFileName = "translations-{repo}-{hash}.csv";
+
+/**
+ * Export existing translation - files into CSV file
+ * @param {string[]} [filesFilter] - fileNames filter, if omitted all files
+ * will be exported
+ */
+function exportTranslations(filesFilter)
+{
+ let mercurialCommands = [];
+ // Get Hash
+ mercurialCommands.push(execFile("hg", ["id", "-i"]));
+ // Get repo path
+ mercurialCommands.push(execFile("hg", ["paths", "default"]));
+ Promise.all(mercurialCommands).then((outputs) =>
+ {
+ // Remove line endings and "+" sign from the end of the hash
+ let [hash, filePath] = outputs.map((item) =>
+ 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.
+ // Update name of the file to be output
+ outputFileName = outputFileName.replace("{hash}", hash);
+ outputFileName = outputFileName.replace("{repo}", path.basename(filePath));
+
+ // Read all available locales and default files
+ 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.
+ }).then((filePaths) =>
+ {
+ // Reading all existing translations files
+ return Promise.all(filePaths.map((filePath) => readJsonPromised(filePath)));
+ }).then(csvFromJsonFileObjects);
+}
+
+/**
+ * Creating Matrix which reflects output CSV file
+ * @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.
+ */
+function csvFromJsonFileObjects(fileObjects)
+{
+ let locales = [];
+ // Create Object tree from the Objects array, for easier search
+ // ex.: {dektop-options.json: {en_US: {...}, {de: {...}, {ru: {...}}}
+ 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.
+ {
+ if (!fileObject)
+ return accumulator;
+
+ let {fileName, locale} = fileObject;
+ if (!locales.includes(locale))
+ locales.push(locale);
+
+ if (!accumulator[fileName])
+ {
+ accumulator[fileName] = {};
+ }
+ accumulator[fileName][locale] = fileObject.strings;
+ return accumulator;
+ }, {});
+
+ let fileNames = Object.keys(dataTreeObj);
+ if (filesFilter.length)
+ fileNames = fileNames.filter((item) => filesFilter.includes(item));
+
+ 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.
+ // Create two dimensional strings array that reflects CSV structure
+ let csvArray = [headers.concat(locales)];
+ for (let fileName of fileNames)
+ {
+ let strings = dataTreeObj[fileName][defaultLocale];
+ for (let stringID of Object.keys(strings))
+ {
+ let fileObj = dataTreeObj[fileName];
+ let {description, message, placeholders} = strings[stringID];
+ let row = [fileName, stringID, description || "",
+ JSON.stringify(placeholders), message];
+
+ for (let locale of locales)
+ {
+ let localeFileObj = fileObj[locale];
+ let isTranslated = !!(localeFileObj && localeFileObj[stringID]);
+ row.push(isTranslated ? localeFileObj[stringID].message : "");
+ }
+ csvArray.push(row);
+ }
+ }
+ arrayToCsv(csvArray);
+}
+
+/**
+ * Import strings from the CSV file
+ * @param {string} filePath - CSV file path to import from
+ */
+function importTranslations(filePath)
+{
+ readFile(filePath, "utf8").then((fileObjects) =>
+ {
+ return csvParser(fileObjects);
+ }).then((dataMatrix) =>
+ {
+ let headLocales = dataMatrix.shift().slice(4);
+ let dataTreeObj = {};
+ for (let rowId in dataMatrix)
+ {
+ let row = dataMatrix[rowId];
+ let [currentFilename, stringId, description, placeholder, ...messages] =
+ row;
+ if (!stringId)
+ continue;
+
+ stringId = stringId.trim();
+ // Check if it's the filename row
+ if (!dataTreeObj[currentFilename])
+ dataTreeObj[currentFilename] = {};
+
+ description = description.trim();
+ for (let i = 0; i < headLocales.length; i++)
+ {
+ let locale = headLocales[i].trim();
+ let message = messages[i].trim();
+ if (!message)
+ continue;
+
+ // Create Object tree from the Objects array, for easier search
+ // ex.: {dektop-options.json: {en_US: {...}, {de: {...}, {ru: {...}}}
+ if (!dataTreeObj[currentFilename][locale])
+ dataTreeObj[currentFilename][locale] = {};
+
+ let localeObj = dataTreeObj[currentFilename][locale];
+ localeObj[stringId] = {};
+ let stringObj = localeObj[stringId];
+
+ // We keep string descriptions only in default locale files
+ if (locale == defaultLocale)
+ stringObj.description = description;
+
+ stringObj.message = message;
+ if (placeholder)
+ stringObj.placeholders = JSON.parse(placeholder);
+ }
+ }
+ writeJson(dataTreeObj);
+ });
+}
+
+/**
+ * Write locale files according to dataTreeObj
+ * @param {Object} dataTreeObj - ex.:
+ * {dektop-options.json: {en_US: {...}, {de: {...}, {ru: {...}}}
+ */
+function writeJson(dataTreeObj)
+{
+ for (let fileName in dataTreeObj)
+ {
+ for (let locale in dataTreeObj[fileName])
+ {
+ let filePath = path.join(localesDir, locale, fileName);
+ 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.
+ let fileString = JSON.stringify(orderedJSON, null, 2);
+
+ // Newline at end of file to match Coding Style
+ if (locale == defaultLocale)
+ fileString += "\n";
+ fs.writeFile(filePath, fileString, "utf8", (err) =>
+ {
+ if (err)
+ {
+ console.error(err);
+ }
+ else
+ {
+ console.log(`Updated: ${filePath}`);
+ }
+ });
+ }
+ }
+}
+
+/**
+ * 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.
+ * @param {Object} unordered - json object
+ * @returns {Object}
+ */
+function orderJSON(unordered)
+{
+ const ordered = {};
+ for (let key of Object.keys(unordered).sort())
+ {
+ ordered[key] = unordered[key];
+ if (unordered[key].placeholders)
+ ordered[key].placeholders = orderJSON(unordered[key].placeholders);
+
+ ordered[key] = unordered[key];
+ }
+ return ordered;
+}
+
+/**
+ * Convert two dimensional array to the CSV file
+ * @param {Array} csvArray - array to convert from
+ */
+function arrayToCsv(csvArray)
+{
+ csv.stringify(csvArray, (err, output) =>
+ {
+ fs.writeFile(outputFileName, output, "utf8", (error) =>
+ {
+ if (!error)
+ console.log(`${outputFileName} is created`);
+ else
+ console.error(error);
+ });
+ });
+}
+
+/**
+ * Reads JSON file and assign filename and locale to it
+ * @param {string} filePath - ex.: "locales/en_US/desktop-options.json"
+ * @param {function} callback - fileName, locale and strings of locale file
+ * Parameters:
+ * * Error message
+ * * Object containing fileName, locale and strings
+ */
+function readJson(filePath, callback)
+{
+ let {dir, base} = path.parse(filePath);
+ fs.readFile(filePath, "utf8", (err, data) =>
+ {
+ if (err)
+ {
+ callback(err);
+ }
+ else
+ {
+ 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.
+ strings: JSON.parse(data)});
+ }
+ });
+}
+
+/**
+ * Exit process and log error message
+ * @param {String} error error message
+ */
+function exitProcess(error)
+{
+ console.error(error);
+ 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.
+}
+
+// CLI
+let helpText = `
+About: Converts locale files between CSV and JSON formats
+Usage: csv-export.js [option] [argument]
+Options:
+ -f [FILENAME] Name of the files to be exported ex.: -f firstRun.json
+ option can be used multiple times.
+ If omitted all files are being exported
+
+ -o [FILENAME] Output filename ex.:
+ -f firstRun.json -o {hash}-firstRun.csv
+ Placeholders:
+ {hash} - Mercurial current revision hash
+ {repo} - Name of the "Default" repository
+ If omitted the output fileName is set to
+ translations-{repo}-{hash}.csv
+
+ -i [FILENAME] Import file path ex: -i issue-reporter.csv
+`;
+
+let argv = process.argv.slice(2);
+let stopExportScript = false;
+// Filter to be used export to the fileNames inside
+let filesFilter = [];
+
+for (let i = 0; i < argv.length; i++)
+{
+ switch (argv[i])
+ {
+ case "-h":
+ console.log(helpText);
+ 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.
+ break;
+ case "-f":
+ // 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.
+ if (!argv[i + 1])
+ {
+ exitProcess("Please specify the input filename");
+ }
+ 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.
+ {
+ filesFilter.push(argv[i + 1]);
+ }
+ break;
+ case "-o":
+ if (!argv[i + 1])
+ {
+ exitProcess("Please specify the output filename");
+ }
+ 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.
+ {
+ outputFileName = argv[i + 1];
+ }
+ break;
+ case "-i":
+ if (!argv[i + 1])
+ {
+ exitProcess("Please specify the import file");
+ }
+ 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.
+ {
+ let importFile = argv[i + 1];
+ importTranslations(importFile);
+ stopExportScript = true;
+ }
+ break;
+ }
+}
+
+if (!stopExportScript)
+ exportTranslations(filesFilter);
« no previous file with comments | « no previous file | package.json » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld