Skip to content

Commit

Permalink
lib: support FORCE_COLOR for non TTY streams
Browse files Browse the repository at this point in the history
PR-URL: #48034
Backport-PR-URL: #48684
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
MoLow authored and danielleadams committed Jul 12, 2023
1 parent a2bfe02 commit c46b31f
Show file tree
Hide file tree
Showing 14 changed files with 110 additions and 40 deletions.
11 changes: 7 additions & 4 deletions lib/internal/console/constructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ const kMaxGroupIndentation = 1000;
// Lazy loaded for startup performance.
let cliTable;

let utilColors;
function lazyUtilColors() {
utilColors ??= require('internal/util/colors');
return utilColors;
}

// Track amount of indentation required via `console.group()`.
const kGroupIndent = Symbol('kGroupIndent');
const kGroupIndentationWidth = Symbol('kGroupIndentWidth');
Expand All @@ -96,7 +102,6 @@ const kUseStdout = Symbol('kUseStdout');
const kUseStderr = Symbol('kUseStderr');

const optionsMap = new SafeWeakMap();

function Console(options /* or: stdout, stderr, ignoreErrors = true */) {
// We have to test new.target here to see if this function is called
// with new, because we need to define a custom instanceof to accommodate
Expand Down Expand Up @@ -315,9 +320,7 @@ ObjectDefineProperties(Console.prototype, {
value: function(stream) {
let color = this[kColorMode];
if (color === 'auto') {
color = stream.isTTY && (
typeof stream.getColorDepth === 'function' ?
stream.getColorDepth() > 2 : true);
color = lazyUtilColors().shouldColorize(stream);
}

const options = optionsMap.get(this);
Expand Down
11 changes: 7 additions & 4 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@ function lazyInternalUtilInspect() {
return internalUtilInspect;
}

let utilColors;
function lazyUtilColors() {
utilColors ??= require('internal/util/colors');
return utilColors;
}

let buffer;
function lazyBuffer() {
buffer ??= require('buffer').Buffer;
Expand Down Expand Up @@ -795,10 +801,7 @@ const fatalExceptionStackEnhancers = {
colors: defaultColors,
},
} = lazyInternalUtilInspect();
const colors = useColors &&
((internalBinding('util').guessHandleType(2) === 'TTY' &&
require('internal/tty').hasColors()) ||
defaultColors);
const colors = useColors && (lazyUtilColors().shouldColorize(process.stderr) || defaultColors);
try {
return inspect(error, {
colors,
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/test_runner/reporter/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ const {
const assert = require('assert');
const Transform = require('internal/streams/transform');
const { inspectWithNoCustomRetry } = require('internal/errors');
const { green, blue, red, white, gray, hasColors } = require('internal/util/colors');
const { green, blue, red, white, gray, shouldColorize } = require('internal/util/colors');
const { kSubtestsFailed } = require('internal/test_runner/test');
const { getCoverageReport } = require('internal/test_runner/utils');

const inspectOptions = { __proto__: null, colors: hasColors, breakLength: Infinity };
const inspectOptions = { __proto__: null, colors: shouldColorize(process.stdout), breakLength: Infinity };

const colors = {
'__proto__': null,
Expand Down
16 changes: 15 additions & 1 deletion lib/internal/util/colors.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
'use strict';

let internalTTy;
function lazyInternalTTY() {
internalTTy ??= require('internal/tty');
return internalTTy;
}

module.exports = {
blue: '',
green: '',
Expand All @@ -8,9 +14,17 @@ module.exports = {
gray: '',
clear: '',
hasColors: false,
shouldColorize(stream) {
if (process.env.FORCE_COLOR !== undefined) {
return lazyInternalTTY().getColorDepth() > 2;
}
return stream?.isTTY && (
typeof stream.getColorDepth === 'function' ?
stream.getColorDepth() > 2 : true);
},
refresh() {
if (process.stderr.isTTY) {
const hasColors = process.stderr.hasColors();
const hasColors = module.exports.shouldColorize(process.stderr);
module.exports.blue = hasColors ? '\u001b[34m' : '';
module.exports.green = hasColors ? '\u001b[32m' : '';
module.exports.white = hasColors ? '\u001b[39m' : '';
Expand Down
8 changes: 7 additions & 1 deletion lib/internal/util/debuglog.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,19 @@ function emitWarningIfNeeded(set) {

const noop = () => {};

let utilColors;
function lazyUtilColors() {
utilColors ??= require('internal/util/colors');
return utilColors;
}

function debuglogImpl(enabled, set) {
if (debugImpls[set] === undefined) {
if (enabled) {
const pid = process.pid;
emitWarningIfNeeded(set);
debugImpls[set] = function debug(...args) {
const colors = process.stderr.hasColors && process.stderr.hasColors();
const colors = lazyUtilColors().shouldColorize(process.stderr);
const msg = formatWithOptions({ colors }, ...args);
const coloredPID = inspect(pid, { colors });
process.stderr.write(format('%s %s: %s\n', set, coloredPID, msg));
Expand Down
7 changes: 2 additions & 5 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ const {
commonPrefix,
} = require('internal/readline/utils');
const { Console } = require('console');
const { shouldColorize } = require('internal/util/colors');
const CJSModule = require('internal/modules/cjs/loader').Module;
let _builtinLibs = ArrayPrototypeFilter(
CJSModule.builtinModules,
Expand Down Expand Up @@ -272,11 +273,7 @@ function REPLServer(prompt,

if (options.terminal && options.useColors === undefined) {
// If possible, check if stdout supports colors or not.
if (options.output.hasColors) {
options.useColors = options.output.hasColors();
} else if (process.env.NODE_DISABLE_COLORS === undefined) {
options.useColors = true;
}
options.useColors = shouldColorize(options.output) || process.env.NODE_DISABLE_COLORS === undefined;
}

// TODO(devsnek): Add a test case for custom eval functions.
Expand Down
4 changes: 2 additions & 2 deletions test/common/assertSnapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ async function assertSnapshot(actual, filename = process.argv[1]) {
* @param {boolean} [options.tty] - whether to spawn the process in a pseudo-tty
* @returns {Promise<void>}
*/
async function spawnAndAssert(filename, transform = (x) => x, { tty = false } = {}) {
async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...options } = {}) {
if (tty && common.isWindows) {
test({ skip: 'Skipping pseudo-tty tests, as pseudo terminals are not available on Windows.' });
return;
}
const flags = common.parseTestFlags(filename);
const executable = tty ? 'tools/pseudo-tty.py' : process.execPath;
const args = tty ? [process.execPath, ...flags, filename] : [...flags, filename];
const { stdout, stderr } = await common.spawnPromisified(executable, args);
const { stdout, stderr } = await common.spawnPromisified(executable, args, options);
await assertSnapshot(transform(`${stdout}${stderr}`), filename);
}

Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/console/force_colors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use strict';

require('../../common');

console.log(123, 'foo', { bar: 'baz' });
1 change: 1 addition & 0 deletions test/fixtures/console/force_colors.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
123 foo { bar: 'baz' }
1 change: 1 addition & 0 deletions test/fixtures/errors/force_colors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error('Should include grayed stack trace')
14 changes: 14 additions & 0 deletions test/fixtures/errors/force_colors.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
*force_colors.js:1
throw new Error('Should include grayed stack trace')
^

Error: Should include grayed stack trace
at Object.<anonymous> (/test*force_colors.js:1:7)
 at Module._compile (node:internal*modules*cjs*loader:1256:14)
 at Module._extensions..js (node:internal*modules*cjs*loader:1310:10)
 at Module.load (node:internal*modules*cjs*loader:1119:32)
 at Module._load (node:internal*modules*cjs*loader:960:12)
 at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:81:12)
 at node:internal*main*run_main_module:23:47

Node.js *
10 changes: 7 additions & 3 deletions test/parallel/test-node-output-console.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import * as fixtures from '../common/fixtures.mjs';
import * as snapshot from '../common/assertSnapshot.js';
import { describe, it } from 'node:test';

const skipForceColors =
process.config.variables.icu_gyp_path !== 'tools/icu/icu-generic.gyp' ||
process.config.variables.node_shared_openssl;

function replaceStackTrace(str) {
return snapshot.replaceStackTrace(str, '$1at *$7\n');
Expand All @@ -22,12 +25,13 @@ describe('console output', { concurrency: true }, () => {
transform: snapshot
.transform(snapshot.replaceWindowsLineEndings, snapshot.replaceWindowsPaths, normalize)
},
];
!skipForceColors ? { name: 'console/force_colors.js', env: { FORCE_COLOR: 1 } } : null,
].filter(Boolean);
const defaultTransform = snapshot
.transform(snapshot.replaceWindowsLineEndings, snapshot.replaceWindowsPaths, replaceStackTrace);
for (const { name, transform } of tests) {
for (const { name, transform, env } of tests) {
it(name, async () => {
await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform);
await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform, { env });
});
}
});
16 changes: 12 additions & 4 deletions test/parallel/test-node-output-errors.mjs
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import '../common/index.mjs';
import * as common from '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs';
import * as snapshot from '../common/assertSnapshot.js';
import * as os from 'node:os';
import { describe, it } from 'node:test';

const skipForceColors =
process.config.variables.icu_gyp_path !== 'tools/icu/icu-generic.gyp' ||
process.config.variables.node_shared_openssl ||
(common.isWindows && (Number(os.release().split('.')[0]) !== 10 || Number(os.release().split('.')[2]) < 14393)); // See https://github.com/nodejs/node/pull/33132


function replaceNodeVersion(str) {
return str.replaceAll(process.version, '*');
}
Expand Down Expand Up @@ -43,10 +50,11 @@ describe('errors output', { concurrency: true }, () => {
{ name: 'errors/throw_in_line_with_tabs.js', transform: errTransform },
{ name: 'errors/throw_non_error.js', transform: errTransform },
{ name: 'errors/promise_always_throw_unhandled.js', transform: promiseTransform },
];
for (const { name, transform } of tests) {
!skipForceColors ? { name: 'errors/force_colors.js', env: { FORCE_COLOR: 1 } } : null,
].filter(Boolean);
for (const { name, transform, env } of tests) {
it(name, async () => {
await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform);
await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform, { env });
});
}
});
42 changes: 28 additions & 14 deletions test/parallel/test-repl-envvars.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

require('../common');
const stream = require('stream');
const { describe, test } = require('node:test');
const REPL = require('internal/repl');
const assert = require('assert');
const inspect = require('util').inspect;
Expand All @@ -18,13 +19,21 @@ const tests = [
env: { NODE_DISABLE_COLORS: '1' },
expected: { terminal: true, useColors: false }
},
{
env: { NODE_DISABLE_COLORS: '1', FORCE_COLOR: '1' },
expected: { terminal: true, useColors: true }
},
{
env: { NODE_NO_READLINE: '1' },
expected: { terminal: false, useColors: false }
},
{
env: { TERM: 'dumb' },
expected: { terminal: true, useColors: false }
expected: { terminal: true, useColors: true }
},
{
env: { TERM: 'dumb', FORCE_COLOR: '1' },
expected: { terminal: true, useColors: true }
},
{
env: { NODE_NO_READLINE: '1', NODE_DISABLE_COLORS: '1' },
Expand Down Expand Up @@ -56,20 +65,25 @@ function run(test) {

Object.assign(process.env, env);

REPL.createInternalRepl(process.env, opts, function(err, repl) {
assert.ifError(err);
return new Promise((resolve) => {
REPL.createInternalRepl(process.env, opts, function(err, repl) {
assert.ifError(err);

assert.strictEqual(repl.terminal, expected.terminal,
`Expected ${inspect(expected)} with ${inspect(env)}`);
assert.strictEqual(repl.useColors, expected.useColors,
`Expected ${inspect(expected)} with ${inspect(env)}`);
assert.strictEqual(repl.replMode, expected.replMode || REPL_MODE_SLOPPY,
`Expected ${inspect(expected)} with ${inspect(env)}`);
for (const key of Object.keys(env)) {
delete process.env[key];
}
repl.close();
assert.strictEqual(repl.terminal, expected.terminal,
`Expected ${inspect(expected)} with ${inspect(env)}`);
assert.strictEqual(repl.useColors, expected.useColors,
`Expected ${inspect(expected)} with ${inspect(env)}`);
assert.strictEqual(repl.replMode, expected.replMode || REPL_MODE_SLOPPY,
`Expected ${inspect(expected)} with ${inspect(env)}`);
for (const key of Object.keys(env)) {
delete process.env[key];
}
repl.close();
resolve();
});
});
}

tests.forEach(run);
describe('REPL environment variables', { concurrency: 1 }, () => {
tests.forEach((testCase) => test(inspect(testCase.env), () => run(testCase)));
});

0 comments on commit c46b31f

Please sign in to comment.