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(suggestions): clarify Unknown command output #2906

Merged
merged 1 commit into from Mar 19, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions lib/cli.js
Expand Up @@ -66,14 +66,12 @@ module.exports = (process) => {
npm.log.level = 'silent'
if (cmd) {
const didYouMean = require('./utils/did-you-mean.js')
console.error(npm.localPrefix)
const suggestions = await didYouMean(npm, npm.localPrefix, cmd)
npm.output(suggestions)
npm.output(`Unknown command: "${cmd}"${suggestions}`)
} else
npm.output(npm.usage)
process.exitCode = 1
} catch (err) {
console.error(err)
errorHandler(err)
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/run-script.js
Expand Up @@ -92,7 +92,7 @@ class RunScript extends BaseCommand {
return

const suggestions = await didYouMean(this.npm, path, event)
throw new Error(suggestions)
throw new Error(`Missing script: "${event}"${suggestions}`)
}

// positional args only added to the main event, not pre/post
Expand Down Expand Up @@ -228,7 +228,7 @@ class RunScript extends BaseCommand {
log.error(` in workspace: ${pkg._id || pkg.name}`)
log.error(` at location: ${workspacePath}`)

const scriptMissing = err.message.startsWith('Unknown command')
const scriptMissing = err.message.startsWith('Missing script')

// avoids exiting with error code in case there's scripts missing
// in some workspaces since other scripts might have succeeded
Expand Down
9 changes: 5 additions & 4 deletions lib/utils/did-you-mean.js
Expand Up @@ -23,10 +23,11 @@ const didYouMean = async (npm, path, scmd) => {

const best = [...bestCmd, ...bestRun, ...bestBin]

const suggestion = best.length === 0 ? ''
: best.length === 1 ? `\n\nDid you mean this?\n${best[0]}`
if (best.length === 0)
return ''

const suggestion = best.length === 1 ? `\n\nDid you mean this?\n${best[0]}`
: `\n\nDid you mean one of these?\n${best.slice(0, 3).join('\n')}`
const result = `Unknown command: "${scmd}"${suggestion}`
return result
return suggestion
}
module.exports = didYouMean
4 changes: 2 additions & 2 deletions test/lib/cli.js
Expand Up @@ -46,7 +46,7 @@ const npmlogMock = {
const requireInject = require('require-inject')
const cli = requireInject.installGlobally('../../lib/cli.js', {
'../../lib/npm.js': npmock,
'../../lib/utils/did-you-mean.js': () => 'test did you mean',
'../../lib/utils/did-you-mean.js': () => '\ntest did you mean',
'../../lib/utils/unsupported.js': unsupportedMock,
'../../lib/utils/error-handler.js': errorHandlerMock,
npmlog: npmlogMock,
Expand Down Expand Up @@ -160,7 +160,7 @@ t.test('print usage if non-command param provided', t => {
npmock.argv = ['asdf']
npmock.output = (msg) => {
if (msg) {
t.match(msg, 'test did you mean', 'outputs did you mean')
t.match(msg, 'Unknown command: "asdf"\ntest did you mean', 'outputs did you mean')
t.end()
}
}
Expand Down
22 changes: 11 additions & 11 deletions test/lib/run-script.js
Expand Up @@ -290,15 +290,15 @@ t.test('try to run missing script', t => {
t.test('no suggestions', t => {
runScript.exec(['notevenclose'], er => {
t.match(er, {
message: 'Unknown command: "notevenclose"',
message: 'Missing script: "notevenclose"',
})
t.end()
})
})
t.test('script suggestions', t => {
runScript.exec(['helo'], er => {
t.match(er, {
message: 'Unknown command: "helo"',
message: 'Missing script: "helo"',
})
t.match(er, {
message: 'npm run hello',
Expand All @@ -309,7 +309,7 @@ t.test('try to run missing script', t => {
t.test('bin suggestions', t => {
runScript.exec(['goodneght'], er => {
t.match(er, {
message: 'Unknown command: "goodneght"',
message: 'Missing script: "goodneght"',
})
t.match(er, {
message: 'npm exec goodnight',
Expand Down Expand Up @@ -896,27 +896,27 @@ t.test('workspaces', t => {
t.match(RUN_SCRIPTS, [])
t.strictSame(LOG.map(cleanOutput), [
'Lifecycle script `missing-script` failed with error:',
'Error: Unknown command: "missing-script"',
'Error: Missing script: "missing-script"',
' in workspace: a@1.0.0',
' at location: {CWD}/test/lib/run-script-workspaces/packages/a',
'Lifecycle script `missing-script` failed with error:',
'Error: Unknown command: "missing-script"',
'Error: Missing script: "missing-script"',
' in workspace: b@2.0.0',
' at location: {CWD}/test/lib/run-script-workspaces/packages/b',
'Lifecycle script `missing-script` failed with error:',
'Error: Unknown command: "missing-script"',
'Error: Missing script: "missing-script"',
' in workspace: c@1.0.0',
' at location: {CWD}/test/lib/run-script-workspaces/packages/c',
'Lifecycle script `missing-script` failed with error:',
'Error: Unknown command: "missing-script"',
'Error: Missing script: "missing-script"',
' in workspace: d@1.0.0',
' at location: {CWD}/test/lib/run-script-workspaces/packages/d',
'Lifecycle script `missing-script` failed with error:',
'Error: Unknown command: "missing-script"',
'Error: Missing script: "missing-script"',
' in workspace: e',
' at location: {CWD}/test/lib/run-script-workspaces/packages/e',
'Lifecycle script `missing-script` failed with error:',
'Error: Unknown command: "missing-script"',
'Error: Missing script: "missing-script"',
' in workspace: noscripts@1.0.0',
' at location: {CWD}/test/lib/run-script-workspaces/packages/noscripts',
], 'should log error msgs for each workspace script')
Expand All @@ -937,11 +937,11 @@ t.test('workspaces', t => {
t.match(RUN_SCRIPTS, [])
t.strictSame(LOG.map(cleanOutput), [
'Lifecycle script `test` failed with error:',
'Error: Unknown command: "test"',
'Error: Missing script: "test"',
' in workspace: a@1.0.0',
' at location: {CWD}/test/lib/run-script-workspaces/packages/a',
'Lifecycle script `test` failed with error:',
'Error: Unknown command: "test"',
'Error: Missing script: "test"',
' in workspace: b@2.0.0',
' at location: {CWD}/test/lib/run-script-workspaces/packages/b',
], 'should log error msgs for each workspace script')
Expand Down
6 changes: 1 addition & 5 deletions test/lib/utils/did-you-mean.js
Expand Up @@ -8,23 +8,20 @@ t.test('did-you-mean', t => {
t.notOk(err)
t.test('nistall', async t => {
const result = await dym(npm, npm.localPrefix, 'nistall')
t.match(result, 'Unknown command')
t.match(result, 'npm install')
})
t.test('sttest', async t => {
const result = await dym(npm, npm.localPrefix, 'sttest')
t.match(result, 'Unknown command')
t.match(result, 'npm test')
t.match(result, 'npm run posttest')
})
t.test('npz', async t => {
const result = await dym(npm, npm.localPrefix, 'npxx')
t.match(result, 'Unknown command')
t.match(result, 'npm exec npx')
})
t.test('qwuijbo', async t => {
const result = await dym(npm, npm.localPrefix, 'qwuijbo')
t.match(result, 'Unknown command')
t.match(result, '')
})
t.end()
})
Expand All @@ -38,6 +35,5 @@ t.test('missing bin and script properties', async t => {
})

const result = await dym(npm, path, 'nistall')
t.match(result, 'Unknown command')
t.match(result, 'npm install')
})