Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework logging colors #3350

Merged
merged 4 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions js/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
}

Expand All @@ -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.`);
}
}

Expand Down
12 changes: 6 additions & 6 deletions js/check_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -33,20 +33,20 @@ 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!");
}

// Check permission
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");
Expand All @@ -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}`);
Expand Down
33 changes: 31 additions & 2 deletions js/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion js/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
9 changes: 1 addition & 8 deletions js/utils.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
/* MagicMirror²
* Utils
*
* By Rodrigo Ramírez Norambuena https://rodrigoramirez.com
* By KristjanESPERANTO https://github.com/KristjanESPERANTO
KristjanESPERANTO marked this conversation as resolved.
Show resolved Hide resolved
* 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: {
rejas marked this conversation as resolved.
Show resolved Hide resolved
warn: colors.yellow,
error: colors.red,
info: colors.blue,
pass: colors.green
},

async logSystemInformation () {
try {
Expand Down
22 changes: 13 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
32 changes: 18 additions & 14 deletions tests/unit/classes/utils_spec.js
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand All @@ -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!");
});
});
});
Loading