From 1c3bd303167d1dace74cce322dc6d8bc17cb0e4e Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 28 Feb 2017 20:56:15 +0100 Subject: [PATCH 01/17] clarify initial part of src/cli/index.js when we extracting arguments; flags and arguments after -- --- src/cli/index.js | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index cf550e95dd..f5d8e107af 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -24,19 +24,12 @@ const pkg = require('../../package.json'); loudRejection(); -// const startArgs = process.argv.slice(0, 2); -let args = process.argv.slice(2); // ignore all arguments after a -- -let endArgs = []; -for (let i = 0; i < args.length; i++) { - const arg = args[i]; - if (arg === '--') { - endArgs = args.slice(i + 1); - args = args.slice(0, i); - } -} +const doubleDashIndex = process.argv.findIndex((element) => element === '--'); +const args = process.argv.slice(2, doubleDashIndex === -1 ? process.argv.length : doubleDashIndex); +const endArgs = doubleDashIndex === -1 ? [] : process.argv.slice(doubleDashIndex + 1, process.argv.length); // NOTE: Pending resolution of https://github.com/tj/commander.js/issues/346 // Remove this (and subsequent use in the logic below) after bug is resolved and issue is closed From 123b5bfc659ba3376a2dea832b417697b5f2535e Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 28 Feb 2017 21:17:59 +0100 Subject: [PATCH 02/17] change test if we do not provide any command name and specify a flag because the previous one was flaky --- __tests__/index.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/__tests__/index.js b/__tests__/index.js index 3c1a2eb370..a5220228b3 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -81,10 +81,6 @@ function expectAnErrorMessage(command: Promise>, error: string) : ); } -function expectInstallOutput(stdout) { - expect(stdout[0]).toEqual(`yarn install v${pkg.version}`); -} - test.concurrent('should add package', async () => { const stdout = await execCommand('add', ['left-pad'], 'run-add', true); expectAddSuccessfullOutput(stdout, 'left-pad'); @@ -182,12 +178,12 @@ test.concurrent('should run --version command', async () => { test.concurrent('should install if no args', async () => { const stdout = await execCommand('', [], 'run-add', true); - expectInstallOutput(stdout); + expect(stdout[0]).toEqual(`yarn install v${pkg.version}`); }); test.concurrent('should install if first arg looks like a flag', async () => { - const stdout = await execCommand('--offline', [], 'run-add', true); - expectInstallOutput(stdout); + const stdout = await execCommand('--json', [], 'run-add', true); + expect(stdout[stdout.length - 1]).toEqual('{"type":"success","data":"Saved lockfile."}'); }); test.concurrent('should interpolate aliases', async () => { From 6ec7f97571c72dcd58bca8a7fe12249195515311 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 28 Feb 2017 21:55:00 +0100 Subject: [PATCH 03/17] fix silent error in test when we expect an error from a yarn command --- __tests__/index.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/__tests__/index.js b/__tests__/index.js index a5220228b3..9dad6c9bc1 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -76,7 +76,11 @@ function expectHelpOutputAsSubcommand(stdout) { } function expectAnErrorMessage(command: Promise>, error: string) : Promise { - return command.catch((reason) => + return command + .then(function() { + throw new Error('the command did not fail'); + }) + .catch((reason) => expect(reason.message).toContain(error), ); } From f2683780c236cab86e8e621881c0e96a394c07b6 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Fri, 10 Mar 2017 14:01:43 +0100 Subject: [PATCH 04/17] enrich commands with aliases --- src/cli/index.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index f5d8e107af..6f7fd6ad99 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -2,7 +2,7 @@ import {ConsoleReporter, JSONReporter} from '../reporters/index.js'; import {registries, registryNames} from '../registries/index.js'; -import * as commands from './commands/index.js'; +import * as _commands from './commands/index.js'; import * as constants from '../constants.js'; import * as network from '../util/network.js'; import {MessageError} from '../errors.js'; @@ -24,6 +24,15 @@ const pkg = require('../../package.json'); loudRejection(); +const commands = {..._commands}; +for (const key in aliases) { + commands[key] = { + run(config: Config, reporter: ConsoleReporter | JSONReporter): Promise { + throw new MessageError(`Did you mean \`yarn ${aliases[key]}\`?`); + }, + }; +} + const startArgs = process.argv.slice(0, 2); // ignore all arguments after a -- @@ -112,16 +121,6 @@ if (!commandName || commandName[0] === '-') { commandName = 'install'; } -// aliases: i -> install -if (commandName && typeof aliases[commandName] === 'string') { - const alias = aliases[commandName]; - command = { - run(config: Config, reporter: ConsoleReporter | JSONReporter): Promise { - throw new MessageError(`Did you mean \`yarn ${alias}\`?`); - }, - }; -} - // if (commandName === 'help' && args.length) { commandName = camelCase(args.shift()); From c9affc74fb9daaa92e8a65dca0b04c3b5a1b3832 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Fri, 10 Mar 2017 14:14:08 +0100 Subject: [PATCH 05/17] simplify if statement: if no command we set install as default --- src/cli/index.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index 6f7fd6ad99..55c9536c3f 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -100,24 +100,21 @@ commander.option('--network-concurrency ', 'maximum number of concurrent commander.option('--network-timeout ', 'TCP timeout for network requests', parseInt); commander.option('--non-interactive', 'do not show interactive prompts'); -// get command name -let commandName: ?string = args.shift() || ''; -let command; - -// const getDocsLink = (name) => `${constants.YARN_DOCS}${name || ''}`; const getDocsInfo = (name) => 'Visit ' + chalk.bold(getDocsLink(name)) + ' for documentation about this command.'; +// get command name +let commandName: ?string = args.shift() || 'install'; +let command; + // if (commandName === '--help' || commandName === '-h') { commandName = 'help'; } // if no args or command name looks like a flag then default to `install` -if (!commandName || commandName[0] === '-') { - if (commandName) { - args.unshift(commandName); - } +if (commandName && commandName[0] === '-') { + args.unshift(commandName); commandName = 'install'; } From 04c3ce1d961adb8c121d3f00a91d132d0d2155fa Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Fri, 10 Mar 2017 14:24:32 +0100 Subject: [PATCH 06/17] simplify if statement: we always have command with value undefined --- src/cli/index.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index 55c9536c3f..39129ab4ee 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -126,11 +126,9 @@ if (commandName === 'help' && args.length) { // invariant(commandName, 'Missing command name'); -if (!command) { - const camelised = camelCase(commandName); - if (camelised) { - command = commands[camelised]; - } +const camelised = camelCase(commandName); +if (camelised) { + command = commands[camelised]; } // From 89cb6655dee26bb384948e2924ba97c814f0e227 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Fri, 10 Mar 2017 14:51:06 +0100 Subject: [PATCH 07/17] put every logic related to help in help command --- src/cli/commands/help.js | 53 ++++++++++++++++++++++++++++------------ src/cli/index.js | 34 ++++++-------------------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/cli/commands/help.js b/src/cli/commands/help.js index c2398cf712..ff96e50397 100644 --- a/src/cli/commands/help.js +++ b/src/cli/commands/help.js @@ -4,7 +4,7 @@ import * as commands from './index.js'; import * as constants from '../../constants.js'; import type {Reporter} from '../../reporters/index.js'; import type Config from '../../config.js'; -import {sortAlpha, hyphenate} from '../../util/misc.js'; +import {sortAlpha, hyphenate, camelCase} from '../../util/misc.js'; const chalk = require('chalk'); export function run( @@ -17,24 +17,47 @@ export function run( const getDocsInfo = (name) => 'Visit ' + chalk.bold(getDocsLink(name)) + ' for documentation about this command.'; if (args.length) { - const helpCommand = hyphenate(args[0]); - if (commands[helpCommand]) { - commander.on('--help', () => console.log(' ' + getDocsInfo(helpCommand) + '\n')); - } - } else { - commander.on('--help', () => { - console.log(' Commands:\n'); - for (const name of Object.keys(commands).sort(sortAlpha)) { - if (commands[name].useless) { - continue; + const commandName = camelCase(args.shift()); + + if (commandName) { + const command = commands[commandName]; + + if (command) { + if (typeof command.setFlags === 'function') { + command.setFlags(commander); } - console.log(` - ${hyphenate(name)}`); + const examples: Array = (command && command.examples) || []; + if (examples.length) { + commander.on('--help', () => { + console.log(' Examples:\n'); + for (const example of examples) { + console.log(` $ yarn ${example}`); + } + console.log(); + }); + } + commander.on('--help', () => console.log(' ' + getDocsInfo(commandName) + '\n')); + + commander.help(); + return Promise.resolve(); } - console.log('\n Run `' + chalk.bold('yarn help COMMAND') + '` for more information on specific commands.'); - console.log(' Visit ' + chalk.bold(getDocsLink()) + ' to learn more about Yarn.\n'); - }); + } } + + commander.on('--help', () => { + console.log(' Commands:\n'); + for (const name of Object.keys(commands).sort(sortAlpha)) { + if (commands[name].useless) { + continue; + } + + console.log(` - ${hyphenate(name)}`); + } + console.log('\n Run `' + chalk.bold('yarn help COMMAND') + '` for more information on specific commands.'); + console.log(' Visit ' + chalk.bold(getDocsLink()) + ' to learn more about Yarn.\n'); + }); + commander.help(); return Promise.resolve(); } diff --git a/src/cli/index.js b/src/cli/index.js index 39129ab4ee..a80dae325a 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -37,7 +37,7 @@ const startArgs = process.argv.slice(0, 2); // ignore all arguments after a -- const doubleDashIndex = process.argv.findIndex((element) => element === '--'); -const args = process.argv.slice(2, doubleDashIndex === -1 ? process.argv.length : doubleDashIndex); +let args = process.argv.slice(2, doubleDashIndex === -1 ? process.argv.length : doubleDashIndex); const endArgs = doubleDashIndex === -1 ? [] : process.argv.slice(doubleDashIndex + 1, process.argv.length); // NOTE: Pending resolution of https://github.com/tj/commander.js/issues/346 @@ -104,26 +104,24 @@ const getDocsLink = (name) => `${constants.YARN_DOCS}${name || ''}`; const getDocsInfo = (name) => 'Visit ' + chalk.bold(getDocsLink(name)) + ' for documentation about this command.'; // get command name -let commandName: ?string = args.shift() || 'install'; +let commandName: string = args.shift() || 'install'; let command; -// if (commandName === '--help' || commandName === '-h') { commandName = 'help'; } +if (args.indexOf('--help') >= 0 || args.indexOf('-h') >= 0) { + args.unshift(commandName); + commandName = 'help'; +} + // if no args or command name looks like a flag then default to `install` if (commandName && commandName[0] === '-') { args.unshift(commandName); commandName = 'install'; } -// -if (commandName === 'help' && args.length) { - commandName = camelCase(args.shift()); - args.push('--help'); -} - // invariant(commandName, 'Missing command name'); const camelised = camelCase(commandName); @@ -136,24 +134,6 @@ if (command && typeof command.setFlags === 'function') { command.setFlags(commander); } -if (args.indexOf('--help') >= 0 || args.indexOf('-h') >= 0) { - const examples: Array = (command && command.examples) || []; - if (examples.length) { - commander.on('--help', () => { - console.log(' Examples:\n'); - for (const example of examples) { - console.log(` $ yarn ${example}`); - } - console.log(); - }); - } - commander.on('--help', () => console.log(' ' + getDocsInfo(commandName) + '\n')); - - commander.parse(startArgs.concat(args)); - commander.help(); - process.exit(1); -} - args = [commandName].concat(getRcArgs(commandName), args); if (ARGS_THAT_SHARE_NAMES_WITH_OPTIONS.indexOf(commandName) >= 0 && args[0] === commandName) { From 0f268f3c91ea0b121bf05606167ffb45092b9565 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Fri, 10 Mar 2017 15:01:41 +0100 Subject: [PATCH 08/17] use deconstructuring instead of concat and some shift/unshit command --- src/cli/index.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index a80dae325a..14e9663710 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -37,7 +37,7 @@ const startArgs = process.argv.slice(0, 2); // ignore all arguments after a -- const doubleDashIndex = process.argv.findIndex((element) => element === '--'); -let args = process.argv.slice(2, doubleDashIndex === -1 ? process.argv.length : doubleDashIndex); +const args = process.argv.slice(2, doubleDashIndex === -1 ? process.argv.length : doubleDashIndex); const endArgs = doubleDashIndex === -1 ? [] : process.argv.slice(doubleDashIndex + 1, process.argv.length); // NOTE: Pending resolution of https://github.com/tj/commander.js/issues/346 @@ -129,18 +129,16 @@ if (camelised) { command = commands[camelised]; } -// if (command && typeof command.setFlags === 'function') { command.setFlags(commander); } -args = [commandName].concat(getRcArgs(commandName), args); - -if (ARGS_THAT_SHARE_NAMES_WITH_OPTIONS.indexOf(commandName) >= 0 && args[0] === commandName) { - args.shift(); -} - -commander.parse(startArgs.concat(args)); +commander.parse([ + ...startArgs, + ARGS_THAT_SHARE_NAMES_WITH_OPTIONS.indexOf(commandName) === -1 ? commandName : '', + ...getRcArgs(commandName), + ...args, +]); commander.args = commander.args.concat(endArgs); if (command) { From 89fdcbcb7f434eb20da753b6eea93195141f8fc5 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Fri, 10 Mar 2017 16:53:05 +0100 Subject: [PATCH 09/17] remove useless invariant on commandName: we are sure that is always defined --- src/cli/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index 14e9663710..f87476aefb 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -117,13 +117,11 @@ if (args.indexOf('--help') >= 0 || args.indexOf('-h') >= 0) { } // if no args or command name looks like a flag then default to `install` -if (commandName && commandName[0] === '-') { +if (commandName[0] === '-') { args.unshift(commandName); commandName = 'install'; } -// -invariant(commandName, 'Missing command name'); const camelised = camelCase(commandName); if (camelised) { command = commands[camelised]; From c227e40661a9beac6e9ecfeee9f78e7e1311e5d1 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Fri, 10 Mar 2017 18:54:42 +0100 Subject: [PATCH 10/17] if command is not recognized set default to run; if command is run we set commandName as the first arg for npm_config_argv --- __tests__/lifecycle-scripts.js | 3 +++ src/cli/index.js | 21 +++++++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/__tests__/lifecycle-scripts.js b/__tests__/lifecycle-scripts.js index 51e77633f1..d7e900b903 100644 --- a/__tests__/lifecycle-scripts.js +++ b/__tests__/lifecycle-scripts.js @@ -67,6 +67,9 @@ async () => { stdout = await execCommand('', 'npm_config_argv_env_vars', env); expect(stdout).toContain('##install##'); + stdout = await execCommand('run test', 'npm_config_argv_env_vars', env); + expect(stdout).toContain('##test##'); + stdout = await execCommand('test', 'npm_config_argv_env_vars', env); expect(stdout).toContain('##test##'); }); diff --git a/src/cli/index.js b/src/cli/index.js index f87476aefb..dcb22b265f 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -105,7 +105,6 @@ const getDocsInfo = (name) => 'Visit ' + chalk.bold(getDocsLink(name)) + ' for d // get command name let commandName: string = args.shift() || 'install'; -let command; if (commandName === '--help' || commandName === '-h') { commandName = 'help'; @@ -116,17 +115,25 @@ if (args.indexOf('--help') >= 0 || args.indexOf('-h') >= 0) { commandName = 'help'; } -// if no args or command name looks like a flag then default to `install` +// if no args or command name looks like a flag then set default to `install` if (commandName[0] === '-') { args.unshift(commandName); commandName = 'install'; } +let command; const camelised = camelCase(commandName); if (camelised) { command = commands[camelised]; } +// if command is not recognized, then set default to `run` +if (!command) { + args.unshift(commandName); + commandName = 'run'; + command = commands.run; +} + if (command && typeof command.setFlags === 'function') { command.setFlags(commander); } @@ -138,13 +145,7 @@ commander.parse([ ...args, ]); commander.args = commander.args.concat(endArgs); - -if (command) { - commander.args.shift(); -} else { - command = commands.run; -} -invariant(command, 'missing command'); +commander.args.shift(); // let Reporter = ConsoleReporter; @@ -345,7 +346,7 @@ config.init({ httpsProxy: commander.httpsProxy, networkConcurrency: commander.networkConcurrency, nonInteractive: commander.nonInteractive, - commandName, + commandName: commandName === 'run' ? commander.args[0] : commandName, }).then(() => { // option "no-progress" stored in yarn config From fbd0c88a85c4f0dd960023d59dc2df46c0dee2ff Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Sat, 11 Mar 2017 12:21:55 +0100 Subject: [PATCH 11/17] add some tests cases when we do not recognize command --- __tests__/index.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/__tests__/index.js b/__tests__/index.js index 9dad6c9bc1..4ca796758e 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -216,3 +216,24 @@ test.concurrent('should run bin command', async () => { expect(stdout[0]).toEqual(path.join(fixturesLoc, 'node_modules', '.bin')); expect(stdout.length).toEqual(1); }); + +test.concurrent('should throws missing command for not camelised command', async () => { + await expectAnErrorMessage( + execCommand('HelP', [], 'run-add', true), + 'Command \"HelP\" not found', + ); +}); + +test.concurrent('should throws missing command for not alphabetic command', async () => { + await expectAnErrorMessage( + execCommand('123', [], 'run-add', true), + 'Command \"123\" not found', + ); +}); + +test.concurrent('should throws missing command for unknown command', async () => { + await expectAnErrorMessage( + execCommand('unknown', [], 'run-add', true), + 'Command \"unknown\" not found', + ); +}); From 33149512956b5d427fa21fcf3d102040d0c50ae1 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Sat, 11 Mar 2017 12:26:43 +0100 Subject: [PATCH 12/17] we use commander.js only to parse flags, remove every logic to put commandName and place only a placeholder --- src/cli/index.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index dcb22b265f..77fbba1526 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -40,12 +40,6 @@ const doubleDashIndex = process.argv.findIndex((element) => element === '--'); const args = process.argv.slice(2, doubleDashIndex === -1 ? process.argv.length : doubleDashIndex); const endArgs = doubleDashIndex === -1 ? [] : process.argv.slice(doubleDashIndex + 1, process.argv.length); -// NOTE: Pending resolution of https://github.com/tj/commander.js/issues/346 -// Remove this (and subsequent use in the logic below) after bug is resolved and issue is closed -const ARGS_THAT_SHARE_NAMES_WITH_OPTIONS = [ - 'version', -]; - // set global options commander.version(pkg.version); commander.usage('[command] [flags]'); @@ -130,7 +124,6 @@ if (camelised) { // if command is not recognized, then set default to `run` if (!command) { args.unshift(commandName); - commandName = 'run'; command = commands.run; } @@ -140,11 +133,15 @@ if (command && typeof command.setFlags === 'function') { commander.parse([ ...startArgs, - ARGS_THAT_SHARE_NAMES_WITH_OPTIONS.indexOf(commandName) === -1 ? commandName : '', + // we use this for https://github.com/tj/commander.js/issues/346, otherwise + // it will strip some args that match with any options + 'this-arg-will-get-stripped-later', ...getRcArgs(commandName), ...args, ]); commander.args = commander.args.concat(endArgs); + +// we strip cmd commander.args.shift(); // From 0899ac7ac6867e3062582fdb4fca5e444026f0fb Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 28 Feb 2017 22:08:58 +0100 Subject: [PATCH 13/17] implement hasWrapper function for help command --- src/cli/commands/help.js | 4 ++++ src/cli/index.js | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cli/commands/help.js b/src/cli/commands/help.js index ff96e50397..a8cb225698 100644 --- a/src/cli/commands/help.js +++ b/src/cli/commands/help.js @@ -7,6 +7,10 @@ import type Config from '../../config.js'; import {sortAlpha, hyphenate, camelCase} from '../../util/misc.js'; const chalk = require('chalk'); +export function hasWrapper(): boolean { + return false; +} + export function run( config: Config, reporter: Reporter, diff --git a/src/cli/index.js b/src/cli/index.js index 77fbba1526..d6532d6d3d 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -168,7 +168,7 @@ if (typeof command.hasWrapper === 'function') { if (commander.json) { outputWrapper = false; } -if (outputWrapper && commandName !== 'help') { +if (outputWrapper) { reporter.header(commandName, pkg); } From 71072dbed54abef63f317b00379e4938eeead01c Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Sat, 11 Mar 2017 12:40:09 +0100 Subject: [PATCH 14/17] display correct help link for aliases --- __tests__/index.js | 25 +++++++++++++++++++++---- src/cli/index.js | 2 +- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/__tests__/index.js b/__tests__/index.js index 4ca796758e..09e6dc29ec 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -26,9 +26,9 @@ Promise> { } return new Promise((resolve, reject) => { - exec(`node "${yarnBin}" ${cmd} ${args.join(' ')}`, {cwd:workingDir, env:process.env}, (err, stdout) => { - if (err) { - reject(err); + exec(`node "${yarnBin}" ${cmd} ${args.join(' ')}`, {cwd:workingDir, env:process.env}, (error, stdout) => { + if (error) { + reject({error, stdout}); } else { const stdoutLines = stdout.toString() .split('\n') @@ -81,7 +81,17 @@ function expectAnErrorMessage(command: Promise>, error: string) : throw new Error('the command did not fail'); }) .catch((reason) => - expect(reason.message).toContain(error), + expect(reason.error.message).toContain(error), + ); +} + +function expectAnInfoMessageAfterError(command: Promise>, info: string) : Promise { + return command + .then(function() { + throw new Error('the command did not fail'); + }) + .catch((reason) => + expect(reason.stdout).toContain(info), ); } @@ -197,6 +207,13 @@ test.concurrent('should interpolate aliases', async () => { ); }); +test.concurrent('should display correct documentation for aliases', async () => { + await expectAnInfoMessageAfterError( + execCommand('i', [], 'run-add', true), + 'Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.', + ); +}); + test.concurrent('should run help of run command if --help is before --', async () => { const stdout = await execCommand('run', ['custom-script', '--help', '--'], 'run-custom-script-with-arguments'); expect(stdout[0]).toEqual('Usage: yarn [command] [flags]'); diff --git a/src/cli/index.js b/src/cli/index.js index d6532d6d3d..e41117611e 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -385,7 +385,7 @@ config.init({ } if (commandName) { - const actualCommandForHelp = commands[commandName] ? commandName : aliases[commandName]; + const actualCommandForHelp = aliases[commandName] ? aliases[commandName] : commandName; if (command && actualCommandForHelp) { reporter.info(getDocsInfo(actualCommandForHelp)); } From 4d930e9c8a51fae8cb7b3084115ab4b8e7c16d71 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Sat, 11 Mar 2017 12:58:55 +0100 Subject: [PATCH 15/17] display documentation link correctly in every case --- __tests__/index.js | 16 +++++++++++++++- src/cli/index.js | 9 ++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/__tests__/index.js b/__tests__/index.js index 09e6dc29ec..52d9ae8de6 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -207,7 +207,7 @@ test.concurrent('should interpolate aliases', async () => { ); }); -test.concurrent('should display correct documentation for aliases', async () => { +test.concurrent('should display correct documentation link for aliases', async () => { await expectAnInfoMessageAfterError( execCommand('i', [], 'run-add', true), 'Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.', @@ -254,3 +254,17 @@ test.concurrent('should throws missing command for unknown command', async () => 'Command \"unknown\" not found', ); }); + +test.concurrent('should not display documentation link for unknown command', async () => { + await expectAnInfoMessageAfterError( + execCommand('unknown', [], 'run-add', true), + '', + ); +}); + +test.concurrent('should display documentation link for known command', async () => { + await expectAnInfoMessageAfterError( + execCommand('add', [], 'run-add', true), + 'Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.', + ); +}); diff --git a/src/cli/index.js b/src/cli/index.js index e41117611e..a72cfd566a 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -384,11 +384,10 @@ config.init({ onUnexpectedError(err); } - if (commandName) { - const actualCommandForHelp = aliases[commandName] ? aliases[commandName] : commandName; - if (command && actualCommandForHelp) { - reporter.info(getDocsInfo(actualCommandForHelp)); - } + if (aliases[commandName]) { + reporter.info(getDocsInfo(aliases[commandName])); + } else if (commands[commandName]) { + reporter.info(getDocsInfo(commandName)); } process.exit(1); From 045bd60acf56de9968d4095c016fce75cbedddf9 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Mon, 10 Apr 2017 16:11:35 +0200 Subject: [PATCH 16/17] add some console.asserts to ensure that we early crash if something is not expected --- src/cli/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cli/index.js b/src/cli/index.js index a72cfd566a..ade84a20e4 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -142,6 +142,8 @@ commander.parse([ commander.args = commander.args.concat(endArgs); // we strip cmd +console.assert(commander.args.length >= 1); +console.assert(commander.args[0] === 'this-arg-will-get-stripped-later'); commander.args.shift(); // From 2fb8a3ed7fc936ecd814ee0c7699652c96618c7b Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Mon, 10 Apr 2017 18:15:53 +0200 Subject: [PATCH 17/17] fix erronous case: yarn constructor --- __tests__/index.js | 7 +++++++ src/cli/index.js | 2 +- src/rc.js | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/__tests__/index.js b/__tests__/index.js index 52d9ae8de6..a4d2d7aa57 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -268,3 +268,10 @@ test.concurrent('should display documentation link for known command', async () 'Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.', ); }); + +test.concurrent('should throws missing command for constructor command', async () => { + await expectAnErrorMessage( + execCommand('constructor', [], 'run-add', true), + 'Command \"constructor\" not found', + ); +}); diff --git a/src/cli/index.js b/src/cli/index.js index ade84a20e4..5909b9f0c7 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -117,7 +117,7 @@ if (commandName[0] === '-') { let command; const camelised = camelCase(commandName); -if (camelised) { +if (camelised && Object.prototype.hasOwnProperty.call(commands, camelised)) { command = commands[camelised]; } diff --git a/src/rc.js b/src/rc.js index 29ae25f4b3..7d22abaff7 100644 --- a/src/rc.js +++ b/src/rc.js @@ -70,7 +70,7 @@ export function getRcArgs(command: string): Array { let result = rcArgsCache['*'] || []; - if (command !== '*') { + if (command !== '*' && Object.prototype.hasOwnProperty.call(rcArgsCache, command)) { result = result.concat(rcArgsCache[command] || []); }