Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: hide banner for exec and explore #7421

Merged
merged 3 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading