From ec903a766d8ae9aea74f8afbe8ba1379e03cb2a4 Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Sun, 14 Jan 2024 22:45:09 +0100 Subject: [PATCH] Rework logging colors - 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) --- CHANGELOG.md | 1 + js/app.js | 8 ++++---- js/check_config.js | 12 ++++++------ js/logger.js | 33 ++++++++++++++++++++++++++++++-- js/server.js | 2 +- js/utils.js | 9 +-------- package-lock.json | 22 ++++++++++++--------- package.json | 2 +- tests/unit/classes/utils_spec.js | 32 +++++++++++++++++-------------- 9 files changed, 76 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 468cc9ba48..bd2b2777b0 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) ### Fixed diff --git a/js/app.js b/js/app.js index 33db688233..086f4ec5c2 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 b3a0fe9429..d190f22a75 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 228c53e999..da295bb990 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 a183fe4df8..03de57ebef 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 b3b2992a69..e84a087f0b 100644 --- a/js/utils.js +++ b/js/utils.js @@ -1,21 +1,14 @@ /* MagicMirror² * Utils * - * By Rodrigo Ramírez Norambuena https://rodrigoramirez.com + * By KristjanESPERANTO https://github.com/KristjanESPERANTO * 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 { diff --git a/package-lock.json b/package-lock.json index 986e535a5a..52a8a40ee6 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", @@ -2132,6 +2132,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", @@ -2925,14 +2937,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 6ecce674bc..19c72f54bc 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "electron": "^27.2.0" }, "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 bf7a5c593a..a91a9e76e7 100644 --- a/tests/unit/classes/utils_spec.js +++ b/tests/unit/classes/utils_spec.js @@ -1,5 +1,9 @@ -const colors = require("colors/safe"); -const Utils = require("../../../js/utils"); +/* + * Note from KristjanESPERANTO: + * TODO: This test no longer tests our own code but only ansis. In my opinion, the color test can be removed, instead we need a test for logSystemInformation. + */ + +const colors = require("ansis"); describe("Utils", () => { describe("colors", () => { @@ -10,29 +14,29 @@ describe("Utils", () => { }); it("should have info, warn and error properties", () => { - expect(Utils.colors).toHaveProperty("info"); - expect(Utils.colors).toHaveProperty("warn"); - expect(Utils.colors).toHaveProperty("error"); + expect(colors).toHaveProperty("info"); + expect(colors).toHaveProperty("warn"); + expect(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"); + expect(typeof colors.info).toBe("function"); + expect(typeof colors.warn).toBe("function"); + expect(typeof 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"); + expect(colors.info("some informations")).toBe("\u001b[34msome informations\u001b[39m"); + expect(colors.warn("a warning")).toBe("\u001b[33ma warning\u001b[39m"); + expect(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!"); + expect(colors.info("some informations")).toBe("some informations"); + expect(colors.warn("a warning")).toBe("a warning"); + expect(colors.error("ERROR!")).toBe("ERROR!"); }); }); });