fix(config): consolidate use of npm.color

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 })

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()
 

lib/ls.js

@@ -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')

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
   }

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 []

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',

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
 }

test/lib/exec.js

@@ -25,6 +25,7 @@ const LOG_WARN = []
 let PROGRESS_IGNORED = false
 const flatOptions = {
   npxCache: 'npx-cache-dir',
+  color: false,
   cache: 'cache-dir',
   legacyPeerDeps: false,
   package: [],
@@ -109,12 +110,13 @@ t.afterEach(() => {
   LOG_WARN.length = 0
   PROGRESS_IGNORED = false
   flatOptions.legacyPeerDeps = false
-  config.color = false
+  flatOptions.color = false
   config['script-shell'] = 'shell-cmd'
   config.package = []
   flatOptions.package = []
   config.call = ''
   config.yes = true
+  npm.color = false
   npm.localBin = 'local-bin'
   npm.globalBin = 'global-bin'
 })
@@ -268,7 +270,8 @@ t.test('npm exec <noargs>, run interactive shell', t => {
   t.test('print message with color when tty and not in CI', t => {
     CI_NAME = null
     process.stdin.isTTY = true
-    config.color = true
+    npm.color = true
+    flatOptions.color = true
 
     run(t, true, () => {
       t.strictSame(LOG_WARN, [])
@@ -1204,7 +1207,8 @@ t.test('workspaces', t => {
       })
     })
 
-    config.color = true
+    npm.color = true
+    flatOptions.color = true
     npm._mockOutputs.length = 0
     await new Promise((res, rej) => {
       exec.execWorkspaces([], ['a'], er => {

test/lib/utils/error-message.js

@@ -15,6 +15,9 @@ const { resolve } = require('path')
 const npm = require('../../../lib/npm.js')
 const CACHE = '/some/cache/dir'
 npm.config = {
+  flat: {
+    color: false,
+  },
   loaded: false,
   localPrefix: '/some/prefix/dir',
   get: key => {
@@ -467,7 +470,7 @@ t.test('explain ERESOLVE errors', t => {
   t.matchSnapshot(errorMessage(er, npm))
   t.match(EXPLAIN_CALLED, [[
     er,
-    undefined,
+    false,
     path.resolve(npm.cache, 'eresolve-report.txt'),
   ]])
   t.end()

test/lib/utils/setup-log.js

@@ -84,12 +84,12 @@ t.test('setup with color=always and unicode', t => {
   t.strictSame(WARN_CALLED, [['ERESOLVE', 'hello', { some: 'object' }]])
   WARN_CALLED.length = 0
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: 'always',
     unicode: true,
     progress: false,
-  })), true)
+  }))
 
   npmlog.warn('ERESOLVE', 'hello', { some: { other: 'object' } })
   t.strictSame(EXPLAIN_CALLED, [[{ some: { other: 'object' } }, true, 2]],
@@ -125,12 +125,12 @@ t.test('setup with color=true, no unicode, and non-TTY terminal', t => {
   process.stderr.isTTY = false
   process.stdout.isTTY = false
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: false,
     progress: false,
     heading: 'asdf',
-  })), false)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',
@@ -156,12 +156,12 @@ t.test('setup with color=true, no unicode, and dumb TTY terminal', t => {
   process.stdout.isTTY = true
   process.env.TERM = 'dumb'
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: true,
     progress: false,
     heading: 'asdf',
-  })), true)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',
@@ -187,12 +187,12 @@ t.test('setup with color=true, no unicode, and non-dumb TTY terminal', t => {
   process.stdout.isTTY = true
   process.env.TERM = 'totes not dum'
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: true,
     progress: true,
     heading: 'asdf',
-  })), true)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',
@@ -218,12 +218,12 @@ t.test('setup with non-TTY stdout, TTY stderr', t => {
   process.stdout.isTTY = false
   process.env.TERM = 'definitely not a dummy'
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: true,
     progress: true,
     heading: 'asdf',
-  })), false)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',
@@ -248,12 +248,12 @@ t.test('setup with TTY stdout, non-TTY stderr', t => {
   process.stderr.isTTY = false
   process.stdout.isTTY = true
 
-  t.equal(setupLog(config({
+  setupLog(config({
     loglevel: 'warn',
     color: true,
     progress: true,
     heading: 'asdf',
-  })), true)
+  }))
 
   t.strictSame(settings, {
     level: 'warn',