From 5feff12bd77297a60abed695cafc6073c339f9fb Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sun, 10 Sep 2017 20:51:27 +0300 Subject: [PATCH 1/8] Fix: fix incorrect argument parsing **Summary** Fixes #4383. This patch makes argument parsing a bit tidier, and starts supporting `yarn --silent custom-script` style commands as initiall intended by #4152. **Test plan** Existing unit tests. --- src/cli/index.js | 58 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index c1d6b98f16..4976279dac 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -93,38 +93,56 @@ export function main({ commander.option('--non-interactive', 'do not show interactive prompts'); commander.option('--scripts-prepend-node-path [bool]', 'prepend the node executable dir to the PATH in scripts'); - // get command name - let commandName: string = args.shift() || 'install'; - // if -v is the first command, then always exit after returning the version - if (commandName === '-v') { + if (args[0] === '-v') { console.log(version.trim()); process.exitCode = 0; return; } - if (commandName === '--help' || commandName === '-h') { - commandName = 'help'; + // get command name + const firstNonFlagIndex = args.findIndex(arg => !arg.startsWith('-')); + let preCommandArgs; + let commandName = ''; + if (firstNonFlagIndex > -1) { + preCommandArgs = args.slice(0, firstNonFlagIndex); + commandName = args[firstNonFlagIndex]; + args = args.slice(firstNonFlagIndex + 1); + } else { + preCommandArgs = args; + args = []; } - if (Object.prototype.hasOwnProperty.call(commands, commandName) && (args[0] === '--help' || args[0] === '-h')) { - args.unshift(commandName); + const isHelp = arg => arg === '--help' || arg === '-h'; + let isKnownCommand = Object.prototype.hasOwnProperty.call(commands, commandName); + const helpInPre = preCommandArgs.findIndex(isHelp); + const helpInArgs = args.findIndex(isHelp); + const setHelpMode = () => { + if (isKnownCommand) { + args.unshift(commandName); + } commandName = 'help'; - } + isKnownCommand = true; + }; - // if no args or command name looks like a flag then set default to `install` - if (commandName[0] === '-') { - args.unshift(commandName); - commandName = 'install'; + if (helpInPre > -1) { + preCommandArgs.splice(helpInPre); + setHelpMode(); + } else if (isKnownCommand && helpInArgs > -1) { + args.splice(helpInArgs); + setHelpMode(); } let command; - if (Object.prototype.hasOwnProperty.call(commands, commandName)) { - command = commands[commandName]; + if (!commandName) { + commandName = 'install'; + isKnownCommand = true; } - // if command is not recognized, then set default to `run` - if (!command) { + if (isKnownCommand) { + command = commands[commandName]; + } else { + // if command is not recognized, then set default to `run` args.unshift(commandName); command = commands.run; } @@ -139,6 +157,8 @@ export function main({ } } + args = [...preCommandArgs, ...args]; + command.setFlags(commander); commander.parse([ ...startArgs, @@ -489,8 +509,8 @@ export function main({ onUnexpectedError(err); } - if (commands[commandName]) { - reporter.info(commands[commandName].getDocsInfo); + if (command.getDocsInfo) { + reporter.info(command.getDocsInfo); } return exit(1); From dba540e11cb4dbffbba80392620364d7c67e2f34 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sun, 10 Sep 2017 21:32:52 +0300 Subject: [PATCH 2/8] Fix bug around help detection --- .../run-custom-script-with-arguments/package.json | 2 +- __tests__/index.js | 10 ++++------ src/cli/index.js | 5 +++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/__tests__/fixtures/index/run-custom-script-with-arguments/package.json b/__tests__/fixtures/index/run-custom-script-with-arguments/package.json index b0d3da8bb5..e71c50895b 100644 --- a/__tests__/fixtures/index/run-custom-script-with-arguments/package.json +++ b/__tests__/fixtures/index/run-custom-script-with-arguments/package.json @@ -3,6 +3,6 @@ "version": "1.0.0", "license": "UNLICENSED", "scripts": { - "custom-script": "print() { echo \"A message from custom script with args \"$@\"\"; }; print " + "custom-script": "node -p -e \"`A message from custom script with args ${process.argv[process.argv.length - 1]}`\" -- " } } diff --git a/__tests__/index.js b/__tests__/index.js index 46b3d7cccd..104cd88604 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -248,12 +248,10 @@ test.concurrent('should run help of run command if --help is before script', asy ); }); -if (process.platform !== 'win32') { - test.concurrent('should run help of custom-script if --help is after script', async () => { - const stdout = await execCommand('run', ['custom-script', '--help'], 'run-custom-script-with-arguments'); - expect(stdout[stdout.length - 2]).toEqual('A message from custom script with args --help'); - }); -} +test.concurrent('should run help of custom-script if --help is after script', async () => { + const stdout = await execCommand('run', ['custom-script', '--help'], 'run-custom-script-with-arguments'); + expect(stdout[stdout.length - 2]).toEqual('A message from custom script with args --help'); +}); test.concurrent('should run bin command', async () => { const stdout = await execCommand('bin', [], '', true); diff --git a/src/cli/index.js b/src/cli/index.js index 4976279dac..fe0cc33ccd 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -113,8 +113,9 @@ export function main({ args = []; } - const isHelp = arg => arg === '--help' || arg === '-h'; let isKnownCommand = Object.prototype.hasOwnProperty.call(commands, commandName); + + const isHelp = arg => arg === '--help' || arg === '-h'; const helpInPre = preCommandArgs.findIndex(isHelp); const helpInArgs = args.findIndex(isHelp); const setHelpMode = () => { @@ -128,7 +129,7 @@ export function main({ if (helpInPre > -1) { preCommandArgs.splice(helpInPre); setHelpMode(); - } else if (isKnownCommand && helpInArgs > -1) { + } else if (isKnownCommand && helpInArgs === 0) { args.splice(helpInArgs); setHelpMode(); } From bd8beadc58f6389c480abe030931a79de2602910 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sun, 10 Sep 2017 22:00:07 +0300 Subject: [PATCH 3/8] bash vs cmd --- .../index/run-custom-script-with-arguments/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/fixtures/index/run-custom-script-with-arguments/package.json b/__tests__/fixtures/index/run-custom-script-with-arguments/package.json index e71c50895b..bf21b01a34 100644 --- a/__tests__/fixtures/index/run-custom-script-with-arguments/package.json +++ b/__tests__/fixtures/index/run-custom-script-with-arguments/package.json @@ -3,6 +3,6 @@ "version": "1.0.0", "license": "UNLICENSED", "scripts": { - "custom-script": "node -p -e \"`A message from custom script with args ${process.argv[process.argv.length - 1]}`\" -- " + "custom-script": "node -p -e \"'A message from custom script with args ' + process.argv[process.argv.length - 1]\" -- " } } From 7d28dfafc6ee3d114415e0ed83179b514cbaaf9b Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 11 Sep 2017 15:52:02 +0100 Subject: [PATCH 4/8] Make script test Node 4 compatible --- .../index/run-custom-script-with-arguments/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/fixtures/index/run-custom-script-with-arguments/package.json b/__tests__/fixtures/index/run-custom-script-with-arguments/package.json index bf21b01a34..d2824f4ed7 100644 --- a/__tests__/fixtures/index/run-custom-script-with-arguments/package.json +++ b/__tests__/fixtures/index/run-custom-script-with-arguments/package.json @@ -3,6 +3,6 @@ "version": "1.0.0", "license": "UNLICENSED", "scripts": { - "custom-script": "node -p -e \"'A message from custom script with args ' + process.argv[process.argv.length - 1]\" -- " + "custom-script": "node echo.js" } } From 8b26be15bb35f465082a2df546dd93aabacb6f8d Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 11 Sep 2017 16:08:17 +0100 Subject: [PATCH 5/8] Add forgotten file --- .../fixtures/index/run-custom-script-with-arguments/echo.js | 1 + 1 file changed, 1 insertion(+) create mode 100644 __tests__/fixtures/index/run-custom-script-with-arguments/echo.js diff --git a/__tests__/fixtures/index/run-custom-script-with-arguments/echo.js b/__tests__/fixtures/index/run-custom-script-with-arguments/echo.js new file mode 100644 index 0000000000..2ca190e5b2 --- /dev/null +++ b/__tests__/fixtures/index/run-custom-script-with-arguments/echo.js @@ -0,0 +1 @@ +console.log(`A message from custom script with args ${process.argv[process.argv.length - 1]}`); \ No newline at end of file From 5a18bf738b8ed4a9fcfeaa41da82f9ac49b44599 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 12 Sep 2017 11:35:26 +0100 Subject: [PATCH 6/8] Handle options taking arguments --- src/cli/index.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index fe0cc33ccd..2c5cb74ffc 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -101,7 +101,14 @@ export function main({ } // get command name - const firstNonFlagIndex = args.findIndex(arg => !arg.startsWith('-')); + const firstNonFlagIndex = args.findIndex((arg, idx, arr) => { + const isOption = arg.startsWith('-'); + const prev = idx > 0 && arr[idx - 1]; + const prevOption = prev && prev.startsWith('-') && commander.optionFor(prev); + const boundToPrevOption = prevOption && prevOption.required; + + return !isOption && !boundToPrevOption; + }); let preCommandArgs; let commandName = ''; if (firstNonFlagIndex > -1) { @@ -114,7 +121,6 @@ export function main({ } let isKnownCommand = Object.prototype.hasOwnProperty.call(commands, commandName); - const isHelp = arg => arg === '--help' || arg === '-h'; const helpInPre = preCommandArgs.findIndex(isHelp); const helpInArgs = args.findIndex(isHelp); From 8ac2e1da9761e3844c52a8d6d52932505ef03fe9 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 12 Sep 2017 12:08:32 +0100 Subject: [PATCH 7/8] Use JSON parsing for tests --- .../fixtures/index/run-custom-script-with-arguments/echo.js | 2 +- __tests__/index.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/__tests__/fixtures/index/run-custom-script-with-arguments/echo.js b/__tests__/fixtures/index/run-custom-script-with-arguments/echo.js index 2ca190e5b2..3da00a457b 100644 --- a/__tests__/fixtures/index/run-custom-script-with-arguments/echo.js +++ b/__tests__/fixtures/index/run-custom-script-with-arguments/echo.js @@ -1 +1 @@ -console.log(`A message from custom script with args ${process.argv[process.argv.length - 1]}`); \ No newline at end of file +console.log(JSON.stringify(process.argv)); \ No newline at end of file diff --git a/__tests__/index.js b/__tests__/index.js index 217b5baafc..ea971483e7 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -255,8 +255,8 @@ test.concurrent('should run help of run command if --help is before script', asy }); test.concurrent('should run help of custom-script if --help is after script', async () => { - const stdout = await execCommand('run', ['custom-script', '--help'], 'run-custom-script-with-arguments'); - expect(stdout[stdout.length - 2]).toEqual('A message from custom script with args --help'); + const stdout = await execCommand('run', ['--silent', 'custom-script', '--help'], 'run-custom-script-with-arguments'); + expect(JSON.parse(stdout)).toContain('--help'); }); test.concurrent('should run bin command', async () => { From 8a4e37126a0aff5e65db057c472aeb0d8c6569cb Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 12 Sep 2017 12:27:38 +0100 Subject: [PATCH 8/8] Why, thank you flow --- __tests__/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/index.js b/__tests__/index.js index ea971483e7..e2c4fe6943 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -256,7 +256,7 @@ test.concurrent('should run help of run command if --help is before script', asy test.concurrent('should run help of custom-script if --help is after script', async () => { const stdout = await execCommand('run', ['--silent', 'custom-script', '--help'], 'run-custom-script-with-arguments'); - expect(JSON.parse(stdout)).toContain('--help'); + expect(JSON.parse(stdout.join('\n'))).toContain('--help'); }); test.concurrent('should run bin command', async () => {