Skip to content

Commit

Permalink
fix: start consolidating color output
Browse files Browse the repository at this point in the history
chalk already has a way to disable color output, so if we don't want
color we can disable it there and always use that instance of chalk.

This was only updated in the two commands that have real tests.  Doing
it in the other places is going to require making their tests real so
that we don't ALSO have to rewrite their tests just to change their
internal code.
  • Loading branch information
wraithgar authored and nlf committed May 3, 2022
1 parent 9ed00dc commit d654e7e
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 32 deletions.
34 changes: 12 additions & 22 deletions lib/commands/doctor.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const cacache = require('cacache')
const chalk = require('chalk')
const fs = require('fs')
const fetch = require('make-fetch-happen')
const table = require('text-table')
Expand Down Expand Up @@ -102,28 +101,19 @@ class Doctor extends BaseCommand {
messages.push(line)
}

const outHead = ['Check', 'Value', 'Recommendation/Notes'].map(
!this.npm.color ? h => h : h => chalk.underline(h)
)
const outHead = ['Check', 'Value', 'Recommendation/Notes'].map(h => this.npm.chalk.underline(h))
let allOk = true
const outBody = messages.map(
!this.npm.color
? item => {
allOk = allOk && item[1]
item[1] = item[1] ? 'ok' : 'not ok'
item[2] = String(item[2])
return item
}
: item => {
allOk = allOk && item[1]
if (!item[1]) {
item[0] = chalk.red(item[0])
item[2] = chalk.magenta(String(item[2]))
}
item[1] = item[1] ? chalk.green('ok') : chalk.red('not ok')
return item
}
)
const outBody = messages.map(item => {
if (!item[1]) {
allOk = false
item[0] = this.npm.chalk.red(item[0])
item[1] = this.npm.chalk.red('not ok')
item[2] = this.npm.chalk.magenta(String(item[2]))
} else {
item[1] = this.npm.chalk.green('ok')
}
return item
})
const outTable = [outHead, ...outBody]
const tableOpts = {
stringLength: s => ansiTrim(s).length,
Expand Down
7 changes: 2 additions & 5 deletions lib/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ const runScript = require('@npmcli/run-script')
const pacote = require('pacote')
const npa = require('npm-package-arg')
const npmFetch = require('npm-registry-fetch')
const chalk = require('chalk')
const replaceInfo = require('../utils/replace-info.js')

const otplease = require('../utils/otplease.js')
Expand Down Expand Up @@ -151,8 +150,6 @@ class Publish extends BaseCommand {
const results = {}
const json = this.npm.config.get('json')
const { silent } = this.npm
const noop = a => a
const color = this.npm.color ? chalk : { green: noop, bold: noop }
await this.setWorkspaces(filters)

for (const [name, workspace] of this.workspaces.entries()) {
Expand All @@ -164,9 +161,9 @@ class Publish extends BaseCommand {
log.warn(
'publish',
`Skipping workspace ${
color.green(name)
this.npm.chalk.green(name)
}, marked as ${
color.bold('private')
this.npm.chalk.bold('private')
}`
)
continue
Expand Down
19 changes: 15 additions & 4 deletions lib/npm.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
const EventEmitter = require('events')
const { resolve, dirname, join } = require('path')
const Config = require('@npmcli/config')
const chalk = require('chalk')
const which = require('which')
const fs = require('@npmcli/fs')

// Patch the global fs module here at the app level
require('graceful-fs').gracefulify(require('fs'))

const { definitions, flatten, shorthands } = require('./utils/config/index.js')
const usage = require('./utils/npm-usage.js')

const which = require('which')
const fs = require('@npmcli/fs')

const LogFile = require('./utils/log-file.js')
const Timers = require('./utils/timers.js')
const Display = require('./utils/display.js')
Expand All @@ -37,6 +36,7 @@ class Npm extends EventEmitter {
#tmpFolder = null
#title = 'npm'
#argvClean = []
#chalk = null

#logFile = new LogFile()
#display = new Display()
Expand Down Expand Up @@ -322,6 +322,17 @@ class Npm extends EventEmitter {
return this.flatOptions.color
}

get chalk () {
if (!this.#chalk) {
let level = chalk.level
if (!this.color) {
level = 0
}
this.#chalk = new chalk.Instance({ level })
}
return this.#chalk
}

get logColor () {
return this.flatOptions.logColor
}
Expand Down
4 changes: 3 additions & 1 deletion test/fixtures/mock-npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ const LoadMockNpm = async (t, {
})

const npm = init ? new Npm() : null
t.teardown(() => npm && npm.unload())
t.teardown(() => {
npm && npm.unload()
})

if (load) {
await npm.load()
Expand Down

0 comments on commit d654e7e

Please sign in to comment.