From 6dbacbb77391543a794fe31cc7cf2533134d292b Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Tue, 16 Jan 2024 21:54:55 +0100 Subject: [PATCH] Rework logging colors (#3350) - Replacing old package `colors` by drop-in replacement `ansis` - Rework `console-stamp` config to show all Log outputs in same color (errors = red, warnings = yellow, debug = blue background (only for the label), info = blue) - This also fixes `npm run config:check` (broken since 6097547c103be1bc89c572e5bd2be50b342967fa) Feel free to let me know if the PR is too big and you want me to do individual PRs for the changes. Before: ![before](https://github.com/MagicMirrorOrg/MagicMirror/assets/35647502/88e48ec3-102c-40f3-9e9b-5d14fe446a43) After: ![after](https://github.com/MagicMirrorOrg/MagicMirror/assets/35647502/4c8c4bad-08c9-46a3-92c9-14b996c13a7d) --------- Co-authored-by: Veeck --- CHANGELOG.md | 1 + js/app.js | 8 ++++---- js/check_config.js | 12 +++++------ js/logger.js | 33 ++++++++++++++++++++++++++++-- js/server.js | 2 +- js/utils.js | 11 +++------- package-lock.json | 22 ++++++++++++-------- package.json | 2 +- tests/unit/classes/utils_spec.js | 35 ++------------------------------ 9 files changed, 62 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fe6df62..29876fa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ _This release is scheduled to be released on 2024-04-01._ - Removing lodash dependency by replacing merge by spread operator (#3339) - Use node prefix for build-in modules (#3340) +- Rework logging colors (#3350) - Update electron to v28 and update other dependencies ### Fixed diff --git a/js/app.js b/js/app.js index 33db6882..086f4ec5 100644 --- a/js/app.js +++ b/js/app.js @@ -126,11 +126,11 @@ function App () { return Object.assign(defaults, c); } catch (e) { if (e.code === "ENOENT") { - Log.error(Utils.colors.error("WARNING! Could not find config file. Please create one. Starting with default configuration.")); + Log.error("WARNING! Could not find config file. Please create one. Starting with default configuration."); } else if (e instanceof ReferenceError || e instanceof SyntaxError) { - Log.error(Utils.colors.error(`WARNING! Could not validate config file. Starting with default configuration. Please correct syntax errors at or above this line: ${e.stack}`)); + Log.error(`WARNING! Could not validate config file. Starting with default configuration. Please correct syntax errors at or above this line: ${e.stack}`); } else { - Log.error(Utils.colors.error(`WARNING! Could not load config file. Starting with default configuration. Error found: ${e}`)); + Log.error(`WARNING! Could not load config file. Starting with default configuration. Error found: ${e}`); } } @@ -148,7 +148,7 @@ function App () { const usedDeprecated = deprecatedOptions.filter((option) => userConfig.hasOwnProperty(option)); if (usedDeprecated.length > 0) { - Log.warn(Utils.colors.warn(`WARNING! Your config is using deprecated options: ${usedDeprecated.join(", ")}. Check README and CHANGELOG for more up-to-date ways of getting the same functionality.`)); + Log.warn(`WARNING! Your config is using deprecated options: ${usedDeprecated.join(", ")}. Check README and CHANGELOG for more up-to-date ways of getting the same functionality.`); } } diff --git a/js/check_config.js b/js/check_config.js index b3a0fe94..d190f22a 100644 --- a/js/check_config.js +++ b/js/check_config.js @@ -7,13 +7,13 @@ */ const path = require("node:path"); const fs = require("node:fs"); +const colors = require("ansis"); const { Linter } = require("eslint"); const linter = new Linter(); const rootPath = path.resolve(`${__dirname}/../`); const Log = require(`${rootPath}/js/logger.js`); -const Utils = require(`${rootPath}/js/utils.js`); /** * Returns a string with path of configuration file. @@ -33,7 +33,7 @@ function checkConfigFile () { // Check if file is present if (fs.existsSync(configFileName) === false) { - Log.error(Utils.colors.error("File not found: "), configFileName); + Log.error(`File not found: ${configFileName}`); throw new Error("No config file present!"); } @@ -41,12 +41,12 @@ function checkConfigFile () { try { fs.accessSync(configFileName, fs.F_OK); } catch (e) { - Log.error(Utils.colors.error(e)); + Log.error(e); throw new Error("No permission to access config file!"); } // Validate syntax of the configuration file. - Log.info(Utils.colors.info("Checking file... "), configFileName); + Log.info("Checking file... ", configFileName); // I'm not sure if all ever is utf-8 const configFile = fs.readFileSync(configFileName, "utf-8"); @@ -59,9 +59,9 @@ function checkConfigFile () { }); if (errors.length === 0) { - Log.info(Utils.colors.pass("Your configuration file doesn't contain syntax errors :)")); + Log.info(colors.green("Your configuration file doesn't contain syntax errors :)")); } else { - Log.error(Utils.colors.error("Your configuration file contains syntax errors :(")); + Log.error(colors.red("Your configuration file contains syntax errors :(")); for (const error of errors) { Log.error(`Line ${error.line} column ${error.column}: ${error.message}`); diff --git a/js/logger.js b/js/logger.js index 228c53e9..da295bb9 100644 --- a/js/logger.js +++ b/js/logger.js @@ -10,10 +10,39 @@ (function (root, factory) { if (typeof exports === "object") { if (process.env.JEST_WORKER_ID === undefined) { + const colors = require("ansis"); + // add timestamps in front of log messages require("console-stamp")(console, { - pattern: "yyyy-mm-dd HH:MM:ss.l", - include: ["debug", "log", "info", "warn", "error"] + format: ":date(yyyy-mm-dd HH:MM:ss.l) :label(7) :msg", + tokens: { + label: (arg) => { + const { method, defaultTokens } = arg; + let label = defaultTokens.label(arg); + if (method === "error") { + label = colors.red(label); + } else if (method === "warn") { + label = colors.yellow(label); + } else if (method === "debug") { + label = colors.bgBlue(label); + } else if (method === "info") { + label = colors.blue(label); + } + return label; + }, + msg: (arg) => { + const { method, defaultTokens } = arg; + let msg = defaultTokens.msg(arg); + if (method === "error") { + msg = colors.red(msg); + } else if (method === "warn") { + msg = colors.yellow(msg); + } else if (method === "info") { + msg = colors.blue(msg); + } + return msg; + } + } }); } // Node, CommonJS-like diff --git a/js/server.js b/js/server.js index a183fe4d..03de57eb 100644 --- a/js/server.js +++ b/js/server.js @@ -62,7 +62,7 @@ function Server (config) { server.listen(port, config.address || "localhost"); if (config.ipWhitelist instanceof Array && config.ipWhitelist.length === 0) { - Log.warn(Utils.colors.warn("You're using a full whitelist configuration to allow for all IPs")); + Log.warn("You're using a full whitelist configuration to allow for all IPs"); } app.use(function (req, res, next) { diff --git a/js/utils.js b/js/utils.js index b3b2992a..395fecef 100644 --- a/js/utils.js +++ b/js/utils.js @@ -1,21 +1,13 @@ /* MagicMirror² * Utils * - * By Rodrigo Ramírez Norambuena https://rodrigoramirez.com * MIT Licensed. */ const execSync = require("node:child_process").execSync; -const colors = require("colors/safe"); const Log = require("logger"); const si = require("systeminformation"); module.exports = { - colors: { - warn: colors.yellow, - error: colors.red, - info: colors.blue, - pass: colors.green - }, async logSystemInformation () { try { @@ -32,6 +24,9 @@ module.exports = { systemDataString += `\n### VERSIONS: electron: ${process.versions.electron}; used node: ${staticData["versions"]["node"]}; installed node: ${installedNodeVersion}; npm: ${staticData["versions"]["npm"]}; pm2: ${staticData["versions"]["pm2"]}`; systemDataString += `\n### OTHER: timeZone: ${Intl.DateTimeFormat().resolvedOptions().timeZone}; ELECTRON_ENABLE_GPU: ${process.env.ELECTRON_ENABLE_GPU}`; Log.info(systemDataString); + + // Return is currently only for jest + return systemDataString; } catch (e) { Log.error(e); } diff --git a/package-lock.json b/package-lock.json index 22e97abb..557bbc0f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "MIT", "dependencies": { - "colors": "^1.4.0", + "ansis": "^2.0.3", "command-exists": "^1.2.9", "console-stamp": "^3.1.2", "envsub": "^4.1.0", @@ -2159,6 +2159,18 @@ "url": "https://github.com/chalk/ansi-styles?sponsor=1" } }, + "node_modules/ansis": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/ansis/-/ansis-2.0.3.tgz", + "integrity": "sha512-tcSGX0mhuDFHsgRrT56xnZ9v2X+TOeKhJ75YopI5OBgyT7tGaG5m6BmeC+6KHjiucfBvUHehQMecHbULIAkFPA==", + "engines": { + "node": ">=12.13" + }, + "funding": { + "type": "patreon", + "url": "https://patreon.com/biodiscus" + } + }, "node_modules/anymatch": { "version": "3.1.3", "resolved": "https://registry.npmjs.org/anymatch/-/anymatch-3.1.3.tgz", @@ -2952,14 +2964,6 @@ "integrity": "sha512-IfEDxwoWIjkeXL1eXcDiow4UbKjhLdq6/EuSVR9GMN7KVH3r9gQ83e73hsz1Nd1T3ijd5xv1wcWRYO+D6kCI2w==", "dev": true }, - "node_modules/colors": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/colors/-/colors-1.4.0.tgz", - "integrity": "sha512-a+UqTh4kgZg/SlGvfbzDHpgRu7AAQOmmqRHJnxhRZICKFUT91brVhNNt58CMWU9PsBbv3PDCZUHbVxuDiH2mtA==", - "engines": { - "node": ">=0.1.90" - } - }, "node_modules/combined-stream": { "version": "1.0.8", "resolved": "https://registry.npmjs.org/combined-stream/-/combined-stream-1.0.8.tgz", diff --git a/package.json b/package.json index 196dde58..fea92522 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "electron": "^28.1.3" }, "dependencies": { - "colors": "^1.4.0", + "ansis": "^2.0.3", "command-exists": "^1.2.9", "console-stamp": "^3.1.2", "envsub": "^4.1.0", diff --git a/tests/unit/classes/utils_spec.js b/tests/unit/classes/utils_spec.js index bf7a5c59..772bdf61 100644 --- a/tests/unit/classes/utils_spec.js +++ b/tests/unit/classes/utils_spec.js @@ -1,38 +1,7 @@ -const colors = require("colors/safe"); const Utils = require("../../../js/utils"); describe("Utils", () => { - describe("colors", () => { - const colorsEnabled = colors.enabled; - - afterEach(() => { - colors.enabled = colorsEnabled; - }); - - it("should have info, warn and error properties", () => { - expect(Utils.colors).toHaveProperty("info"); - expect(Utils.colors).toHaveProperty("warn"); - expect(Utils.colors).toHaveProperty("error"); - }); - - it("properties should be functions", () => { - expect(typeof Utils.colors.info).toBe("function"); - expect(typeof Utils.colors.warn).toBe("function"); - expect(typeof Utils.colors.error).toBe("function"); - }); - - it("should print colored message in supported consoles", () => { - colors.enabled = true; - expect(Utils.colors.info("some informations")).toBe("\u001b[34msome informations\u001b[39m"); - expect(Utils.colors.warn("a warning")).toBe("\u001b[33ma warning\u001b[39m"); - expect(Utils.colors.error("ERROR!")).toBe("\u001b[31mERROR!\u001b[39m"); - }); - - it("should print message in unsupported consoles", () => { - colors.enabled = false; - expect(Utils.colors.info("some informations")).toBe("some informations"); - expect(Utils.colors.warn("a warning")).toBe("a warning"); - expect(Utils.colors.error("ERROR!")).toBe("ERROR!"); - }); + it("should output system information", async () => { + await expect(Utils.logSystemInformation()).resolves.toContain("platform: linux"); }); });