diff options
author | Gar <gar+gh@danger.computer> | 2021-07-20 21:03:38 +0300 |
---|---|---|
committer | Gar <gar+gh@danger.computer> | 2021-07-22 16:58:09 +0300 |
commit | eb67054c8303348b25f9717c8f82c8d8d494a242 (patch) | |
tree | 6cac6c23c64cbfc3432da8b904d92650a95adf3a /lib | |
parent | 009ad1e683aa061d7e5c78b9362b0bd1b14ee643 (diff) |
fix(config): consolidate use of npm.color
The config value for `color` should never be directly read by npm, or
its submodules. The derived value `npm.color` or
`npm.flatOptions.color` is what we want to use.
This PR consolidates the use of these values, makes sure there is only
one place the value is actually calculated, and stops relying on
duplicated code in the setup-log.js file for setting one of them.
PR-URL: https://github.com/npm/cli/pull/3563
Credit: @wraithgar
Close: #3563
Reviewed-by: @lukekarrys
Diffstat (limited to 'lib')
-rw-r--r-- | lib/exec.js | 4 | ||||
-rw-r--r-- | lib/fund.js | 2 | ||||
-rw-r--r-- | lib/ls.js | 2 | ||||
-rw-r--r-- | lib/npm.js | 8 | ||||
-rw-r--r-- | lib/run-script.js | 2 | ||||
-rw-r--r-- | lib/utils/config/definitions.js | 3 | ||||
-rw-r--r-- | lib/utils/setup-log.js | 3 |
7 files changed, 15 insertions, 9 deletions
diff --git a/lib/exec.js b/lib/exec.js index 959fab666..8c64c2f24 100644 --- a/lib/exec.js +++ b/lib/exec.js @@ -68,7 +68,6 @@ class Exec extends BaseCommand { async _exec (_args, { locationMsg, path, runPath }) { const args = [..._args] const call = this.npm.config.get('call') - const color = this.npm.config.get('color') const { flatOptions, localBin, @@ -87,7 +86,6 @@ class Exec extends BaseCommand { ...flatOptions, args, call, - color, localBin, locationMsg, log, @@ -103,7 +101,7 @@ class Exec extends BaseCommand { async _execWorkspaces (args, filters) { await this.setWorkspaces(filters) - const color = this.npm.config.get('color') + const color = this.npm.color for (const path of this.workspacePaths) { const locationMsg = await getLocationMsg({ color, path }) diff --git a/lib/fund.js b/lib/fund.js index 92580a756..1e0fa1ecb 100644 --- a/lib/fund.js +++ b/lib/fund.js @@ -109,7 +109,7 @@ class Fund extends ArboristWorkspaceCmd { } printHuman (fundingInfo) { - const color = !!this.npm.color + const color = this.npm.color const unicode = this.npm.config.get('unicode') const seenUrls = new Map() @@ -67,7 +67,7 @@ class LS extends ArboristWorkspaceCmd { async ls (args) { const all = this.npm.config.get('all') - const color = !!this.npm.color + const color = this.npm.color const depth = this.npm.config.get('depth') const dev = this.npm.config.get('dev') const development = this.npm.config.get('development') diff --git a/lib/npm.js b/lib/npm.js index db3559a38..966d11210 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -232,7 +232,7 @@ const npm = module.exports = new class extends EventEmitter { process.emit('timeEnd', 'npm:load:setTitle') process.emit('time', 'npm:load:setupLog') - this.color = setupLog(this.config) + setupLog(this.config) process.emit('timeEnd', 'npm:load:setupLog') process.env.COLOR = this.color ? '1' : '0' @@ -261,6 +261,12 @@ const npm = module.exports = new class extends EventEmitter { return flat } + get color () { + // This is a special derived value that takes into consideration not only + // the config, but whether or not we are operating in a tty. + return this.flatOptions.color + } + get lockfileVersion () { return 2 } diff --git a/lib/run-script.js b/lib/run-script.js index b94d2fce0..1daaeb990 100644 --- a/lib/run-script.js +++ b/lib/run-script.js @@ -137,7 +137,7 @@ class RunScript extends BaseCommand { path = path || this.npm.localPrefix const { scripts, name, _id } = await rpj(`${path}/package.json`) const pkgid = _id || name - const color = !!this.npm.color + const color = this.npm.color if (!scripts) return [] diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js index abe6bda70..36b8a84a6 100644 --- a/lib/utils/config/definitions.js +++ b/lib/utils/config/definitions.js @@ -438,6 +438,9 @@ define('cidr', { flatten, }) +// This should never be directly used, the flattened value is the derived value +// and is sent to other modules, and is also exposed as `npm.color` for use +// inside npm itself. define('color', { default: !process.env.NO_COLOR || process.env.NO_COLOR === '0', usage: '--color|--no-color|--color always', diff --git a/lib/utils/setup-log.js b/lib/utils/setup-log.js index 9ee79d192..aaf7fa47e 100644 --- a/lib/utils/setup-log.js +++ b/lib/utils/setup-log.js @@ -18,6 +18,7 @@ module.exports = (config) => { const stderrTTY = process.stderr.isTTY const dumbTerm = process.env.TERM === 'dumb' const stderrNotDumb = stderrTTY && !dumbTerm + // this logic is duplicated in the config 'color' flattener const enableColorStderr = color === 'always' ? true : color === false ? false : stderrTTY @@ -58,6 +59,4 @@ module.exports = (config) => { log.enableProgress() else log.disableProgress() - - return enableColorStdout } |