From 4a0ec20a35aff521c3a08e42b3905012a6566c00 Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Wed, 28 Aug 2024 15:00:11 -0300 Subject: [PATCH] lib: respect terminal capabilities on styleText This PR changes styleText API to respect terminal capabilities and environment variables such as NO_COLOR, NODE_DISABLE_COLORS, and FORCE_COLOR. PR-URL: https://github.com/nodejs/node/pull/54389 Fixes: https://github.com/nodejs/node/issues/54365 Reviewed-By: Moshe Atlow Reviewed-By: Claudio Wunder Reviewed-By: Rich Trott --- doc/api/util.md | 43 +++++++++++++-- lib/util.js | 42 +++++++++++++- test/parallel/test-bootstrap-modules.js | 1 + test/parallel/test-util-styletext.js | 73 +++++++++++++++++++++++-- 4 files changed, 146 insertions(+), 13 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index bc7c7da964776d..6c1cffd9a1c64f 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -1802,7 +1802,7 @@ console.log(util.stripVTControlCharacters('\u001B[4mvalue\u001B[0m')); // Prints "value" ``` -## `util.styleText(format, text)` +## `util.styleText(format, text[, options])` > Stability: 1.1 - Active development @@ -1810,24 +1810,55 @@ console.log(util.stripVTControlCharacters('\u001B[4mvalue\u001B[0m')); added: - v21.7.0 - v20.12.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/54389 + description: Respect isTTY and environment variables + such as NO_COLORS, NODE_DISABLE_COLORS, and FORCE_COLOR. --> * `format` {string | Array} A text format or an Array of text formats defined in `util.inspect.colors`. * `text` {string} The text to to be formatted. +* `options` {Object} + * `validateStream` {boolean} When true, `stream` is checked to see if it can handle colors. **Default:** `true`. + * `stream` {Stream} A stream that will be validated if it can be colored. **Default:** `process.stdout`. -This function returns a formatted text considering the `format` passed. +This function returns a formatted text considering the `format` passed +for printing in a terminal, it is aware of the terminal's capabilities +and act according to the configuration set via `NO_COLORS`, +`NODE_DISABLE_COLORS` and `FORCE_COLOR` environment variables. ```mjs import { styleText } from 'node:util'; -const errorMessage = styleText('red', 'Error! Error!'); -console.log(errorMessage); +import { stderr } from 'node:process'; + +const successMessage = styleText('green', 'Success!'); +console.log(successMessage); + +const errorMessage = styleText( + 'red', + 'Error! Error!', + // Validate if process.stderr has TTY + { stream: stderr }, +); +console.error(successMessage); ``` ```cjs const { styleText } = require('node:util'); -const errorMessage = styleText('red', 'Error! Error!'); -console.log(errorMessage); +const { stderr } = require('node:process'); + +const successMessage = styleText('green', 'Success!'); +console.log(successMessage); + +const errorMessage = styleText( + 'red', + 'Error! Error!', + // Validate if process.stderr has TTY + { stream: stderr }, +); +console.error(successMessage); ``` `util.inspect.colors` also provides text formats such as `italic`, and diff --git a/lib/util.js b/lib/util.js index e3f3b11bda8333..25f7ed4bb8a52e 100644 --- a/lib/util.js +++ b/lib/util.js @@ -56,12 +56,25 @@ const { } = require('internal/util/inspect'); const { debuglog } = require('internal/util/debuglog'); const { + validateBoolean, validateFunction, validateNumber, validateString, validateOneOf, } = require('internal/validators'); +const { + isReadableStream, + isWritableStream, + isNodeStream, +} = require('internal/streams/utils'); const types = require('internal/util/types'); + +let utilColors; +function lazyUtilColors() { + utilColors ??= require('internal/util/colors'); + return utilColors; +} + const binding = internalBinding('util'); const { @@ -92,10 +105,25 @@ function escapeStyleCode(code) { /** * @param {string | string[]} format * @param {string} text + * @param {object} [options={}] + * @param {boolean} [options.validateStream=true] - Whether to validate the stream. + * @param {Stream} [options.stream=process.stdout] - The stream used for validation. * @returns {string} */ -function styleText(format, text) { +function styleText(format, text, { validateStream = true, stream = process.stdout } = {}) { validateString(text, 'text'); + validateBoolean(validateStream, 'options.validateStream'); + + if (validateStream) { + if ( + !isReadableStream(stream) && + !isWritableStream(stream) && + !isNodeStream(stream) + ) { + throw new ERR_INVALID_ARG_TYPE('stream', ['ReadableStream', 'WritableStream', 'Stream'], stream); + } + } + if (ArrayIsArray(format)) { let left = ''; let right = ''; @@ -115,6 +143,18 @@ function styleText(format, text) { if (formatCodes == null) { validateOneOf(format, 'format', ObjectKeys(inspect.colors)); } + + // Check colorize only after validating arg type and value + if ( + validateStream && + ( + !stream || + !lazyUtilColors().shouldColorize(stream) + ) + ) { + return text; + } + return `${escapeStyleCode(formatCodes[0])}${text}${escapeStyleCode(formatCodes[1])}`; } diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index f1303515f6585c..d8a55fae95c290 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -46,6 +46,7 @@ expected.beforePreExec = new Set([ 'NativeModule internal/assert', 'NativeModule internal/util/inspect', 'NativeModule internal/util/debuglog', + 'NativeModule internal/streams/utils', 'NativeModule internal/timers', 'NativeModule events', 'Internal Binding buffer', diff --git a/test/parallel/test-util-styletext.js b/test/parallel/test-util-styletext.js index 6baa6a60eac8ac..764ce6f1a31c94 100644 --- a/test/parallel/test-util-styletext.js +++ b/test/parallel/test-util-styletext.js @@ -1,7 +1,12 @@ 'use strict'; -require('../common'); -const assert = require('assert'); -const util = require('util'); + +const common = require('../common'); +const assert = require('node:assert'); +const util = require('node:util'); +const { WriteStream } = require('node:tty'); + +const styled = '\u001b[31mtest\u001b[39m'; +const noChange = 'test'; [ undefined, @@ -31,13 +36,69 @@ assert.throws(() => { code: 'ERR_INVALID_ARG_VALUE', }); -assert.strictEqual(util.styleText('red', 'test'), '\u001b[31mtest\u001b[39m'); +assert.strictEqual( + util.styleText('red', 'test', { validateStream: false }), + '\u001b[31mtest\u001b[39m', +); + +assert.strictEqual( + util.styleText(['bold', 'red'], 'test', { validateStream: false }), + '\u001b[1m\u001b[31mtest\u001b[39m\u001b[22m', +); -assert.strictEqual(util.styleText(['bold', 'red'], 'test'), '\u001b[1m\u001b[31mtest\u001b[39m\u001b[22m'); -assert.strictEqual(util.styleText(['bold', 'red'], 'test'), util.styleText('bold', util.styleText('red', 'test'))); +assert.strictEqual( + util.styleText(['bold', 'red'], 'test', { validateStream: false }), + util.styleText( + 'bold', + util.styleText('red', 'test', { validateStream: false }), + { validateStream: false }, + ), +); assert.throws(() => { util.styleText(['invalid'], 'text'); }, { code: 'ERR_INVALID_ARG_VALUE', }); + +assert.throws(() => { + util.styleText('red', 'text', { stream: {} }); +}, { + code: 'ERR_INVALID_ARG_TYPE', +}); + +// does not throw +util.styleText('red', 'text', { stream: {}, validateStream: false }); + +assert.strictEqual( + util.styleText('red', 'test', { validateStream: false }), + styled, +); + +const fd = common.getTTYfd(); +if (fd !== -1) { + const writeStream = new WriteStream(fd); + + const originalEnv = process.env; + [ + { isTTY: true, env: {}, expected: styled }, + { isTTY: false, env: {}, expected: noChange }, + { isTTY: true, env: { NODE_DISABLE_COLORS: '1' }, expected: noChange }, + { isTTY: true, env: { NO_COLOR: '1' }, expected: noChange }, + { isTTY: true, env: { FORCE_COLOR: '1' }, expected: styled }, + { isTTY: true, env: { FORCE_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: styled }, + { isTTY: false, env: { FORCE_COLOR: '1', NO_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: styled }, + { isTTY: true, env: { FORCE_COLOR: '1', NO_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: styled }, + ].forEach((testCase) => { + writeStream.isTTY = testCase.isTTY; + process.env = { + ...process.env, + ...testCase.env + }; + const output = util.styleText('red', 'test', { stream: writeStream }); + assert.strictEqual(output, testCase.expected); + process.env = originalEnv; + }); +} else { + common.skip('Could not create TTY fd'); +}