Skip to content

Commit

Permalink
fix: hide banner for exec and explore (#7421)
Browse files Browse the repository at this point in the history
Fixes #7419
  • Loading branch information
lukekarrys committed Apr 25, 2024
1 parent 57ebebf commit 71cbd91
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 21 deletions.
39 changes: 23 additions & 16 deletions lib/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,7 @@ class Npm {
}

async load () {
return time.start('npm:load', async () => {
const { exec } = await this.#load()
return {
exec,
command: this.argv.shift(),
args: this.argv,
}
})
return time.start('npm:load', () => this.#load())
}

get loaded () {
Expand Down Expand Up @@ -165,7 +158,26 @@ class Npm {

await time.start('npm:load:configload', () => this.config.load())

// npm --versions
if (this.config.get('versions', 'cli')) {
this.argv = ['version']
this.config.set('usage', false, 'cli')
} else {
this.argv = [...this.config.parsedArgv.remain]
}

// Remove first argv since that is our command as typed
// Note that this might not be the actual name of the command
// due to aliases, etc. But we use the raw form of it later
// in user output so it must be preserved as is.
const commandArg = this.argv.shift()

// This is the actual name of the command that will be run or
// undefined if deref could not find a match
const command = deref(commandArg)

await this.#display.load({
command,
loglevel: this.config.get('loglevel'),
stdoutColor: this.color,
stderrColor: this.logColor,
Expand Down Expand Up @@ -202,9 +214,10 @@ class Npm {

// note: this MUST be shorter than the actual argv length, because it
// uses the same memory, so node will truncate it if it's too long.
// We time this because setting process.title is slow sometimes but we
// have to do it for security reasons. But still helpful to know how slow it is.
time.start('npm:load:setTitle', () => {
const { parsedArgv: { cooked, remain } } = this.config
this.argv = remain
// Secrets are mostly in configs, so title is set using only the positional args
// to keep those from being leaked. We still do a best effort replaceInfo.
this.#title = ['npm'].concat(replaceInfo(remain)).join(' ').trim()
Expand Down Expand Up @@ -244,13 +257,7 @@ class Npm {
log.warn('using --force', 'Recommended protections disabled.')
}

// npm --versions
if (this.config.get('versions', 'cli')) {
this.argv = ['version']
this.config.set('usage', false, 'cli')
}

return { exec: true }
return { exec: true, command: commandArg, args: this.argv }
}

get isShellout () {
Expand Down
15 changes: 11 additions & 4 deletions lib/utils/display.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ class Display {
#progress

// options
#command
#levelIndex
#timing
#json
Expand Down Expand Up @@ -159,6 +160,7 @@ class Display {
}

async load ({
command,
heading,
json,
loglevel,
Expand All @@ -168,6 +170,7 @@ class Display {
timing,
unicode,
}) {
this.#command = command
// get createSupportsColor from chalk directly if this lands
// https://github.com/chalk/chalk/pull/600
const [{ Chalk }, { createSupportsColor }] = await Promise.all([
Expand Down Expand Up @@ -272,10 +275,14 @@ class Display {
return
}

// HACK: if it looks like the banner and we are silent do not print it.
// There's no other way to do this right now :(
// eslint-disable-next-line max-len
if (this.#silent && args.length === 1 && args[0].startsWith('\n> ') && args[0].endsWith('\n')) {
// HACK: if it looks like the banner and we are in a state where we hide the
// banner then dont write any output. This hack can be replaced with proc-log.META
const isBanner = args.length === 1 &&
typeof args[0] === 'string' &&
args[0].startsWith('\n> ') &&
args[0].endsWith('\n')
const hideBanner = this.#silent || ['exec', 'explore'].includes(this.#command)
if (isBanner && hideBanner) {
return
}

Expand Down
2 changes: 1 addition & 1 deletion test/lib/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ t.test('basic usage', async t => {
// are generated in the following test
const { npm } = await loadMockNpm(t, {
mocks: {
'{LIB}/utils/cmd-list.js': { commands: [] },
'{LIB}/utils/cmd-list.js': { ...cmdList, commands: [] },
},
config: { userconfig: '/some/config/file/.npmrc' },
globals: { process: { platform: 'posix' } },
Expand Down

0 comments on commit 71cbd91

Please sign in to comment.