Skip to content

Commit 05693ac

Browse files
MoLowMoLow
authored
lib: support FORCE_COLOR for non TTY streams
PR-URL: #48034 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent c18a455 commit 05693ac

14 files changed

+110
-40
lines changed

‎lib/internal/console/constructor.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,12 @@ const kMaxGroupIndentation = 1000;
7979
// Lazy loaded for startup performance.
8080
let cliTable;
8181

82+
let utilColors;
83+
function lazyUtilColors() {
84+
utilColors ??= require('internal/util/colors');
85+
return utilColors;
86+
}
87+
8288
// Track amount of indentation required via `console.group()`.
8389
const kGroupIndent = Symbol('kGroupIndent');
8490
const kGroupIndentationWidth = Symbol('kGroupIndentWidth');
@@ -95,7 +101,6 @@ const kUseStdout = Symbol('kUseStdout');
95101
const kUseStderr = Symbol('kUseStderr');
96102

97103
const optionsMap = new SafeWeakMap();
98-
99104
function Console(options /* or: stdout, stderr, ignoreErrors = true */) {
100105
// We have to test new.target here to see if this function is called
101106
// with new, because we need to define a custom instanceof to accommodate
@@ -314,9 +319,7 @@ ObjectDefineProperties(Console.prototype, {
314319
value: function(stream) {
315320
let color = this[kColorMode];
316321
if (color === 'auto') {
317-
color = stream.isTTY && (
318-
typeof stream.getColorDepth === 'function' ?
319-
stream.getColorDepth() > 2 : true);
322+
color = lazyUtilColors().shouldColorize(stream);
320323
}
321324

322325
const options = optionsMap.get(this);

‎lib/internal/errors.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,12 @@ function lazyInternalUtilInspect() {
190190
return internalUtilInspect;
191191
}
192192

193+
let utilColors;
194+
function lazyUtilColors() {
195+
utilColors ??= require('internal/util/colors');
196+
return utilColors;
197+
}
198+
193199
let buffer;
194200
function lazyBuffer() {
195201
buffer ??= require('buffer').Buffer;
@@ -795,10 +801,7 @@ const fatalExceptionStackEnhancers = {
795801
colors: defaultColors,
796802
},
797803
} = lazyInternalUtilInspect();
798-
const colors = useColors &&
799-
((internalBinding('util').guessHandleType(2) === 'TTY' &&
800-
require('internal/tty').hasColors()) ||
801-
defaultColors);
804+
const colors = useColors && (lazyUtilColors().shouldColorize(process.stderr) || defaultColors);
802805
try {
803806
return inspect(error, {
804807
colors,

‎lib/internal/test_runner/reporter/spec.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ const {
1414
const assert = require('assert');
1515
const Transform = require('internal/streams/transform');
1616
const { inspectWithNoCustomRetry } = require('internal/errors');
17-
const { green, blue, red, white, gray, hasColors } = require('internal/util/colors');
17+
const { green, blue, red, white, gray, shouldColorize } = require('internal/util/colors');
1818
const { kSubtestsFailed } = require('internal/test_runner/test');
1919
const { getCoverageReport } = require('internal/test_runner/utils');
2020

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

2323
const colors = {
2424
'__proto__': null,

‎lib/internal/util/colors.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
'use strict';
22

3+
let internalTTy;
4+
function lazyInternalTTY() {
5+
internalTTy ??= require('internal/tty');
6+
return internalTTy;
7+
}
8+
39
module.exports = {
410
blue: '',
511
green: '',
@@ -8,9 +14,17 @@ module.exports = {
814
gray: '',
915
clear: '',
1016
hasColors: false,
17+
shouldColorize(stream) {
18+
if (process.env.FORCE_COLOR !== undefined) {
19+
return lazyInternalTTY().getColorDepth() > 2;
20+
}
21+
return stream?.isTTY && (
22+
typeof stream.getColorDepth === 'function' ?
23+
stream.getColorDepth() > 2 : true);
24+
},
1125
refresh() {
1226
if (process.stderr.isTTY) {
13-
const hasColors = process.stderr.hasColors();
27+
const hasColors = module.exports.shouldColorize(process.stderr);
1428
module.exports.blue = hasColors ? '\u001b[34m' : '';
1529
module.exports.green = hasColors ? '\u001b[32m' : '';
1630
module.exports.white = hasColors ? '\u001b[39m' : '';

‎lib/internal/util/debuglog.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,19 @@ function emitWarningIfNeeded(set) {
4545

4646
const noop = () => {};
4747

48+
let utilColors;
49+
function lazyUtilColors() {
50+
utilColors ??= require('internal/util/colors');
51+
return utilColors;
52+
}
53+
4854
function debuglogImpl(enabled, set) {
4955
if (debugImpls[set] === undefined) {
5056
if (enabled) {
5157
const pid = process.pid;
5258
emitWarningIfNeeded(set);
5359
debugImpls[set] = function debug(...args) {
54-
const colors = process.stderr.hasColors && process.stderr.hasColors();
60+
const colors = lazyUtilColors().shouldColorize(process.stderr);
5561
const msg = formatWithOptions({ colors }, ...args);
5662
const coloredPID = inspect(pid, { colors });
5763
process.stderr.write(format('%s %s: %s\n', set, coloredPID, msg));

‎lib/repl.js

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ const {
121121
commonPrefix,
122122
} = require('internal/readline/utils');
123123
const { Console } = require('console');
124+
const { shouldColorize } = require('internal/util/colors');
124125
const CJSModule = require('internal/modules/cjs/loader').Module;
125126
let _builtinLibs = ArrayPrototypeFilter(
126127
CJSModule.builtinModules,
@@ -270,11 +271,7 @@ function REPLServer(prompt,
270271

271272
if (options.terminal && options.useColors === undefined) {
272273
// If possible, check if stdout supports colors or not.
273-
if (options.output.hasColors) {
274-
options.useColors = options.output.hasColors();
275-
} else if (process.env.NODE_DISABLE_COLORS === undefined) {
276-
options.useColors = true;
277-
}
274+
options.useColors = shouldColorize(options.output) || process.env.NODE_DISABLE_COLORS === undefined;
278275
}
279276

280277
// TODO(devsnek): Add a test case for custom eval functions.

‎test/common/assertSnapshot.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,15 @@ async function assertSnapshot(actual, filename = process.argv[1]) {
5454
* @param {boolean} [options.tty] - whether to spawn the process in a pseudo-tty
5555
* @returns {Promise<void>}
5656
*/
57-
async function spawnAndAssert(filename, transform = (x) => x, { tty = false } = {}) {
57+
async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...options } = {}) {
5858
if (tty && common.isWindows) {
5959
test({ skip: 'Skipping pseudo-tty tests, as pseudo terminals are not available on Windows.' });
6060
return;
6161
}
6262
const flags = common.parseTestFlags(filename);
6363
const executable = tty ? 'tools/pseudo-tty.py' : process.execPath;
6464
const args = tty ? [process.execPath, ...flags, filename] : [...flags, filename];
65-
const { stdout, stderr } = await common.spawnPromisified(executable, args);
65+
const { stdout, stderr } = await common.spawnPromisified(executable, args, options);
6666
await assertSnapshot(transform(`${stdout}${stderr}`), filename);
6767
}
6868

‎test/fixtures/console/force_colors.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
'use strict';
2+
3+
require('../../common');
4+
5+
console.log(123, 'foo', { bar: 'baz' });

‎test/fixtures/console/force_colors.snapshot

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
123 foo { bar: 'baz' }

‎test/fixtures/errors/force_colors.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
throw new Error('Should include grayed stack trace')

‎test/fixtures/errors/force_colors.snapshot

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
*force_colors.js:1
2+
throw new Error('Should include grayed stack trace')
3+
^
4+
5+
Error: Should include grayed stack trace
6+
at Object.<anonymous> (/test*force_colors.js:1:7)
7+
 at Module._compile (node:internal*modules*cjs*loader:1255:14)
8+
 at Module._extensions..js (node:internal*modules*cjs*loader:1309:10)
9+
 at Module.load (node:internal*modules*cjs*loader:1113:32)
10+
 at Module._load (node:internal*modules*cjs*loader:960:12)
11+
 at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:83:12)
12+
 at node:internal*main*run_main_module:23:47
13+
14+
Node.js *

‎test/parallel/test-node-output-console.mjs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import * as fixtures from '../common/fixtures.mjs';
33
import * as snapshot from '../common/assertSnapshot.js';
44
import { describe, it } from 'node:test';
55

6+
const skipForceColors =
7+
process.config.variables.icu_gyp_path !== 'tools/icu/icu-generic.gyp' ||
8+
process.config.variables.node_shared_openssl;
69

710
function replaceStackTrace(str) {
811
return snapshot.replaceStackTrace(str, '$1at *$7\n');
@@ -22,12 +25,13 @@ describe('console output', { concurrency: true }, () => {
2225
transform: snapshot
2326
.transform(snapshot.replaceWindowsLineEndings, snapshot.replaceWindowsPaths, normalize)
2427
},
25-
];
28+
!skipForceColors ? { name: 'console/force_colors.js', env: { FORCE_COLOR: 1 } } : null,
29+
].filter(Boolean);
2630
const defaultTransform = snapshot
2731
.transform(snapshot.replaceWindowsLineEndings, snapshot.replaceWindowsPaths, replaceStackTrace);
28-
for (const { name, transform } of tests) {
32+
for (const { name, transform, env } of tests) {
2933
it(name, async () => {
30-
await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform);
34+
await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform, { env });
3135
});
3236
}
3337
});

‎test/parallel/test-node-output-errors.mjs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
1-
import '../common/index.mjs';
1+
import * as common from '../common/index.mjs';
22
import * as fixtures from '../common/fixtures.mjs';
33
import * as snapshot from '../common/assertSnapshot.js';
4+
import * as os from 'node:os';
45
import { describe, it } from 'node:test';
56

7+
const skipForceColors =
8+
process.config.variables.icu_gyp_path !== 'tools/icu/icu-generic.gyp' ||
9+
process.config.variables.node_shared_openssl ||
10+
(common.isWindows && (Number(os.release().split('.')[0]) !== 10 || Number(os.release().split('.')[2]) < 14393)); // See https://github.com/nodejs/node/pull/33132
11+
12+
613
function replaceNodeVersion(str) {
714
return str.replaceAll(process.version, '*');
815
}
@@ -43,10 +50,11 @@ describe('errors output', { concurrency: true }, () => {
4350
{ name: 'errors/throw_in_line_with_tabs.js', transform: errTransform },
4451
{ name: 'errors/throw_non_error.js', transform: errTransform },
4552
{ name: 'errors/promise_always_throw_unhandled.js', transform: promiseTransform },
46-
];
47-
for (const { name, transform } of tests) {
53+
!skipForceColors ? { name: 'errors/force_colors.js', env: { FORCE_COLOR: 1 } } : null,
54+
].filter(Boolean);
55+
for (const { name, transform, env } of tests) {
4856
it(name, async () => {
49-
await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform);
57+
await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform, { env });
5058
});
5159
}
5260
});

‎test/parallel/test-repl-envvars.js

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
require('../common');
66
const stream = require('stream');
7+
const { describe, test } = require('node:test');
78
const REPL = require('internal/repl');
89
const assert = require('assert');
910
const inspect = require('util').inspect;
@@ -18,13 +19,21 @@ const tests = [
1819
env: { NODE_DISABLE_COLORS: '1' },
1920
expected: { terminal: true, useColors: false }
2021
},
22+
{
23+
env: { NODE_DISABLE_COLORS: '1', FORCE_COLOR: '1' },
24+
expected: { terminal: true, useColors: true }
25+
},
2126
{
2227
env: { NODE_NO_READLINE: '1' },
2328
expected: { terminal: false, useColors: false }
2429
},
2530
{
2631
env: { TERM: 'dumb' },
27-
expected: { terminal: true, useColors: false }
32+
expected: { terminal: true, useColors: true }
33+
},
34+
{
35+
env: { TERM: 'dumb', FORCE_COLOR: '1' },
36+
expected: { terminal: true, useColors: true }
2837
},
2938
{
3039
env: { NODE_NO_READLINE: '1', NODE_DISABLE_COLORS: '1' },
@@ -56,20 +65,25 @@ function run(test) {
5665

5766
Object.assign(process.env, env);
5867

59-
REPL.createInternalRepl(process.env, opts, function(err, repl) {
60-
assert.ifError(err);
68+
return new Promise((resolve) => {
69+
REPL.createInternalRepl(process.env, opts, function(err, repl) {
70+
assert.ifError(err);
6171

62-
assert.strictEqual(repl.terminal, expected.terminal,
63-
`Expected ${inspect(expected)} with ${inspect(env)}`);
64-
assert.strictEqual(repl.useColors, expected.useColors,
65-
`Expected ${inspect(expected)} with ${inspect(env)}`);
66-
assert.strictEqual(repl.replMode, expected.replMode || REPL_MODE_SLOPPY,
67-
`Expected ${inspect(expected)} with ${inspect(env)}`);
68-
for (const key of Object.keys(env)) {
69-
delete process.env[key];
70-
}
71-
repl.close();
72+
assert.strictEqual(repl.terminal, expected.terminal,
73+
`Expected ${inspect(expected)} with ${inspect(env)}`);
74+
assert.strictEqual(repl.useColors, expected.useColors,
75+
`Expected ${inspect(expected)} with ${inspect(env)}`);
76+
assert.strictEqual(repl.replMode, expected.replMode || REPL_MODE_SLOPPY,
77+
`Expected ${inspect(expected)} with ${inspect(env)}`);
78+
for (const key of Object.keys(env)) {
79+
delete process.env[key];
80+
}
81+
repl.close();
82+
resolve();
83+
});
7284
});
7385
}
7486

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

0 commit comments

Comments
 (0)