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(cli): improve handling of forwarding args/flags #101

Merged
merged 1 commit into from
Feb 14, 2017
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
9 changes: 9 additions & 0 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,15 @@
"contributions": [
"bug"
]
},
{
"login": "addityasingh",
"name": "Aditya Pratap Singh",
"avatar_url": "https://avatars.githubusercontent.com/u/5351262?v=3",
"profile": "http://blog.adityapsingh.com",
"contributions": [
"review"
]
}
]
}
79 changes: 43 additions & 36 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ All the benefits of npm scripts without the cost of a bloated package.json and l
[![downloads][downloads-badge]][npm-stat]
[![MIT License][license-badge]][LICENSE]

[![All Contributors](https://img.shields.io/badge/all_contributors-22-orange.svg?style=flat-square)](#contributors)
[![All Contributors](https://img.shields.io/badge/all_contributors-23-orange.svg?style=flat-square)](#contributors)
[![PRs Welcome][prs-badge]][prs]
[![Donate][donate-badge]][donate]
[![Code of Conduct][coc-badge]][coc]
Expand Down Expand Up @@ -58,6 +58,7 @@ module.exports = {
},
// learn more about concurrently here: https://npm.im/concurrently
validate: 'concurrently "nps lint" "nps test" "nps build"',
// concurrently script too verbose for your liking? Check out other/EXAMPLES.md!
},
}
```
Expand All @@ -81,27 +82,41 @@ scripts:
```

To use `p-s`, it's recommended that you either install it globally (`npm i -g p-s`) or add `./node_modules/bin` to your
`$PATH` (be careful that you know what you're doing when doing this).
`$PATH` (be careful that you know what you're doing when doing this, find out how [here](https://youtu.be/2WZ5iS_3Jgs)).

Then you can run:

```console
nps --help
nps help
```

Which will output:

```console
Usage: nps [options]

Options:

-h, --help output usage information
-V, --version output the version number
-s, --silent Silent nps output
-c, --config <filepath> Config file to use (defaults to nearest package-scripts.yml or package-scripts.js)
-l, --log-level <level> The log level to use (error, warn, info [default])
-r, --require <module> Module to preload
Usage: nps [options] <script>...

Commands:
init automatically migrate from npm scripts to p-s
completion generate bash completion script

Options:
--config, -c Config file to use (defaults to nearest package-scripts.yml
or package-scripts.js)
[default: "<path-to-your-project>/package-scripts.js"]
--silent, -s Silent nps output [boolean] [default: false]
--log-level, -l The log level to use
[choices: "error", "warn", "info", "debug"] [default: "info"]
--require, -r Module to preload
-h, --help Show help [boolean]
-v, --version Show version number [boolean]

Examples:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Examples really shine through 👍

nps.js test build Runs the `test` script then the
`build` script
nps.js "test --cover" "build --prod" Runs the `test` script and forwards
the "--cover" flag then the `build`
script and forwards the "--prod"
flag

Available scripts (camel or kebab case accepted)

Expand Down Expand Up @@ -179,14 +194,7 @@ npm install --global p-s

From here you can use `p-s` on the command line via one of the installed aliases: `nps` or `p-s`.

If you do this, you may also be interested in installing the shell autocompletion script. Do so by running:

```
nps completion <optionally-your-bash-profile-file>
```

The bash profile file defaults to `~/.bash_profile` for bash and `~/.zshrc` for zsh. Special thanks to the
[`omelette`][omelette] package for making this so easy.
If you do this, you may also be interested in installing the shell autocompletion script. See more about this below.

## Getting started

Expand Down Expand Up @@ -222,13 +230,12 @@ As indicated above, this will migrate your npm scripts to package-scripts.

##### completion

Installs autocompletion functionality into your default bash or zsh configuration. You can override the default by
providing a specific file:

```console
nps completion ~/.bashrc
nps completion >> <your-bash-profile-file>
```

Normally `<your-bash-profile-file>` will be `~/.bash_profile`, `~/.bashrc`, or `~/.zshrc`.

Note: you should probably only do this if you have the package installed globally. In that case you should probably also
normally use the `nps` alias rather than `p-s` because it's easier to type.

Expand Down Expand Up @@ -266,32 +273,33 @@ Specify the log level to use
You can specify a module which will be loaded before the config file is loaded. This allows you to preload for example
babel-register so you can use all babel presets you like.

##### args
##### scripts

You can pass additional arguments to the script(s) that are being spawned:
To run a script, you simply provide the name of the script like so:

```console
nps lint --fix # --fix will be passed on to the lint script
nps cover
```

##### scripts

To run a script, you simply provide the name of the script like so:
And you can run multiple scripts in series by simply adding more space-separated arguments.

```console
nps cover
nps cover check-coverage
```

And you can run multiple scripts in series by providing a comma-separated list:
And you can pass arguments to scripts by putting the scripts in quotes:

```console
nps cover,check-coverage
nps "test --cover" check-coverage
```

That's all for the CLI.

### package-scripts.js

> Remember, this file is JavaScript, so you can write functions to make things more simple!
> See other/EXAMPLES.md for examples of cool things you can do with this.

`nps` expects to your `package-scripts.js` file to `module.exports` an object with the following properties:

#### scripts
Expand Down Expand Up @@ -410,7 +418,7 @@ Thanks goes to these people ([emoji key][emojis]):
| :---: | :---: | :---: | :---: | :---: | :---: | :---: |
| [<img src="https://avatars.githubusercontent.com/u/1972567?v=3" width="100px;"/><br /><sub>Tommy</sub>](http://www.tommyleunen.com)<br />[🐛](https://github.com/kentcdodds/p-s/issues?q=author%3Atleunen) [💻](https://github.com/kentcdodds/p-s/commits?author=tleunen) [⚠️](https://github.com/kentcdodds/p-s/commits?author=tleunen) 👀 | [<img src="https://avatars.githubusercontent.com/u/509946?v=3" width="100px;"/><br /><sub>Jayson Harshbarger</sub>](http://www.hypercubed.com)<br />💡 👀 | [<img src="https://avatars.githubusercontent.com/u/1355481?v=3" width="100px;"/><br /><sub>JD Isaacks</sub>](http://www.jisaacks.com)<br />[💻](https://github.com/kentcdodds/p-s/commits?author=jisaacks) [⚠️](https://github.com/kentcdodds/p-s/commits?author=jisaacks) | [<img src="https://avatars.githubusercontent.com/u/924465?v=3" width="100px;"/><br /><sub>Christopher Hiller</sub>](https://boneskull.com)<br />👀 | [<img src="https://avatars.githubusercontent.com/u/1834413?v=3" width="100px;"/><br /><sub>Robin Malfait</sub>](https://robinmalfait.com)<br />💡 | [<img src="https://avatars.githubusercontent.com/u/622118?v=3" width="100px;"/><br /><sub>Eric McCormick</sub>](https://ericmccormick.io)<br />👀 [📖](https://github.com/kentcdodds/p-s/commits?author=edm00se) | [<img src="https://avatars.githubusercontent.com/u/1913805?v=3" width="100px;"/><br /><sub>Sam Verschueren</sub>](https://twitter.com/SamVerschueren)<br />👀 |
| [<img src="https://avatars.githubusercontent.com/u/1155589?v=3" width="100px;"/><br /><sub>Sorin Muntean</sub>](https://github.com/sxn)<br />[💻](https://github.com/kentcdodds/p-s/commits?author=sxn) [⚠️](https://github.com/kentcdodds/p-s/commits?author=sxn) [📖](https://github.com/kentcdodds/p-s/commits?author=sxn) | [<img src="https://avatars.githubusercontent.com/u/1970063?v=3" width="100px;"/><br /><sub>Keith Gunn</sub>](https://github.com/gunnx)<br />[🐛](https://github.com/kentcdodds/p-s/issues?q=author%3Agunnx) [💻](https://github.com/kentcdodds/p-s/commits?author=gunnx) [⚠️](https://github.com/kentcdodds/p-s/commits?author=gunnx) | [<img src="https://avatars.githubusercontent.com/u/1019478?v=3" width="100px;"/><br /><sub>Joe Martella</sub>](http://martellaj.github.io)<br />[🐛](https://github.com/kentcdodds/p-s/issues?q=author%3Amartellaj) [💻](https://github.com/kentcdodds/p-s/commits?author=martellaj) [⚠️](https://github.com/kentcdodds/p-s/commits?author=martellaj) | [<img src="https://avatars.githubusercontent.com/u/1887854?v=3" width="100px;"/><br /><sub>Martin Segado</sub>](https://github.com/msegado)<br />[📖](https://github.com/kentcdodds/p-s/commits?author=msegado) | [<img src="https://avatars.githubusercontent.com/u/36491?v=3" width="100px;"/><br /><sub>Bram Borggreve</sub>](http://colmena.io/)<br />[🐛](https://github.com/kentcdodds/p-s/issues?q=author%3Abeeman) [💻](https://github.com/kentcdodds/p-s/commits?author=beeman) | [<img src="https://avatars.githubusercontent.com/u/86454?v=3" width="100px;"/><br /><sub>Elijah Manor</sub>](http://elijahmanor.com)<br />📹 | [<img src="https://avatars.githubusercontent.com/u/10691183?v=3" width="100px;"/><br /><sub>Ragu Ramaswamy</sub>](https://github.com/rrag)<br />[💻](https://github.com/kentcdodds/p-s/commits?author=rrag) [⚠️](https://github.com/kentcdodds/p-s/commits?author=rrag) |
| [<img src="https://avatars.githubusercontent.com/u/2915616?v=3" width="100px;"/><br /><sub>Erik Fox</sub>](http://www.erikfox.co/)<br />[🐛](https://github.com/kentcdodds/p-s/issues?q=author%3Aerikfox) |
| [<img src="https://avatars.githubusercontent.com/u/2915616?v=3" width="100px;"/><br /><sub>Erik Fox</sub>](http://www.erikfox.co/)<br />[🐛](https://github.com/kentcdodds/p-s/issues?q=author%3Aerikfox) | [<img src="https://avatars.githubusercontent.com/u/5351262?v=3" width="100px;"/><br /><sub>Aditya Pratap Singh</sub>](http://blog.adityapsingh.com)<br />👀 |
<!-- ALL-CONTRIBUTORS-LIST:END -->

This project follows the [all-contributors][all-contributors] specification.
Expand Down Expand Up @@ -455,4 +463,3 @@ MIT
[scripty]: https://npmjs.com/package/scripty
[npm scripts]: https://docs.npmjs.com/misc/scripts
[video]: http://kcd.im/p-s-video
[omelette]: https://npmjs.com/package/omelette
41 changes: 14 additions & 27 deletions cli-test/__snapshots__/cli.test.js.snap
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
exports[`test with --get-yargs-completions 1`] = `
Object {
"stderr": "",
"stdout": "like
lint
lint.sub.thing
lint.sub.hiddenThing
",
}
`;

exports[`test with --require 1`] = `
Object {
"stderr": "",
"stdout": "nps executing: echo \"log\"
"stdout": "nps executing: echo \"log\"
log
",
}
Expand All @@ -17,7 +28,7 @@ Object {

exports[`test with a missing config 1`] = `
Object {
"stderr": "Unable to find JS config at \"./something-that-does-not-exist.js\". Attempted to require as \"<projectRootDir>/cli-test/fixtures/something-that-does-not-exist.js\" https://github.com/kentcdodds/p-s/blob/v0.0.0-semantically-released/other/ERRORS_AND_WARNINGS.md#unable-to-find-config
"stderr": "Unable to find JS config at \"./something-that-does-not-exist.js\". Attempted to require as \"<projectRootDir>/cli-test/fixtures/something-that-does-not-exist.js\" https://github.com/kentcdodds/p-s/blob/v0.0.0-semantically-released/other/ERRORS_AND_WARNINGS.md#unable-to-find-config
",
"stdout": "",
}
Expand All @@ -26,32 +37,8 @@ Object {
exports[`test with config with default script 1`] = `
Object {
"stderr": "",
"stdout": "nps executing: echo \"default script\"
"stdout": "nps executing: echo \"default script\"
default script
",
}
`;

exports[`test without arguments 1`] = `
Object {
"stderr": "",
"stdout": "
Usage: nps [options]

Options:

-h, --help output usage information
-V, --version output the version number
-s, --silent Silent nps output
-c, --config <filepath> Config file to use (defaults to nearest package-scripts.yml or package-scripts.js)
-l, --log-level <level> The log level to use (error, warn, info [default])
-r, --require <module> Module to preload

Available scripts (camel or kebab case accepted)

test - echo \"test script\"
lint - echo \"lint.default\"
lint.sub.thing - this is a description - echo \"deeply nested thing\"
",
}
`;
10 changes: 5 additions & 5 deletions cli-test/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@ import runNPS from './run-nps'

const fixturesPath = resolve(__dirname, './fixtures')

test(
'without arguments',
() => snapshot(),
)

test(
'with config with default script',
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this because yargs help output is non-deterministic and based on the width of the terminal.

() => snapshot('-c ./package-scripts-with-default.js'),
Expand All @@ -28,6 +23,11 @@ test(
() => snapshot('--config ./es6-package-scripts.js --require babel-register log'),
)

test(
'with --get-yargs-completions',
() => snapshot('--config ./package-scripts.js --get-yargs-completions li'),
)

function snapshot(args) {
return runNPS(fixturesPath, args).then(results => {
const snapshottableResults = relativizePaths(results)
Expand Down
2 changes: 2 additions & 0 deletions cli-test/fixtures/package-scripts.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
module.exports = {
scripts: {
test: 'echo "test script"',
like: 'echo "I like you"',
let: 'things',
lint: {
default: 'echo "lint.default"',
sub: {
Expand Down
4 changes: 3 additions & 1 deletion cli-test/run-nps.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ function runNPS(cwd, args = '') {
let stdout = ''
let stderr = ''
const command = `node ${NPS_PATH} ${args}`
const child = spawn(command, {cwd})
const env = Object.assign({}, process.env)
delete env.FORCE_COLOR
const child = spawn(command, {cwd, env})

child.on('error', error => {
reject(error)
Expand Down
10 changes: 9 additions & 1 deletion other/ERRORS_AND_WARNINGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ This happens when you use the `--require` flag and the module you specify cannot

## Failed with exit code

This means that one of the scripts p-s tried to run resulted in a non-zero exit code (a failing exit code)
This means that one of the scripts `p-s` tried to run resulted in a non-zero exit code (a failing exit code)

### To Fix:

Expand All @@ -53,3 +53,11 @@ Your `package-scripts.js`, `package-scripts.yml`, or whatever you specified as t
### To Fix:

Make sure that your config is an object or a function that returns an object.

## Invalid flags

This happens if you pass flags to `p-s` that are not valid (like `p-s --invalid-flag-name`). This most often happens when you're trying to forward arguments to a script like: `p-s build --fast`

### To Fix:

Make sure you put your scripts and the relevant arguments in quotes: `p-s "build --fast"`
1 change: 0 additions & 1 deletion other/EXAMPLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ module.exports = {
}

function concurrent(scripts) {
process.env.FORCE_COLOR = true // this is necessary until https://github.com/kimmobrunfeldt/concurrently/issues/86
const names = scripts.join(',')
const quotedScripts = `"nps ${scripts.join('" "nps ')}"`
return `concurrently --prefix "[{name}]" --names "${names}" ${quotedScripts}`
Expand Down
21 changes: 16 additions & 5 deletions package-scripts.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
const transpile = 'babel --copy-files --out-dir dist --ignore *.test.js,fixtures src'
const cleanDist = 'rimraf dist'
const validate = ['build.andValidate', 'test', 'lint']
// const npsBin = 'babel-node src/bin/nps.js'
const npsBin = 'nps'
const validate = [
'build.andValidate',
'test',
'lint',
]

module.exports = {
scripts: {
commit: {
description: 'This uses commitizen to help us generate beautifully formatted commit messages',
script: 'git-cz',
},
temp: 'sleep 8; exit 0',
test: {
default: 'jest --config=test/jest.src.config.json --coverage',
watch: 'jest --config=test/jest.src.config.json --watch',
Expand All @@ -26,7 +33,7 @@ module.exports = {
},
andValidate: {
description: 'Runs the normal build first, then validates the CLI',
script: 'nps build,test.cli',
script: 'nps build && nps test.cli',
},
},
lint: {
Expand Down Expand Up @@ -60,8 +67,12 @@ module.exports = {
}

function concurrent(scripts) {
process.env.FORCE_COLOR = true // this is required until https://github.com/kimmobrunfeldt/concurrently/issues/86
const names = scripts.join(',')
const quotedScripts = `"nps ${scripts.join('" "nps ')}"`
return `concurrently --prefix "[{name}]" --names "${names}" ${quotedScripts}`
const quotedScripts = `"${npsBin} ${scripts.join(`" "${npsBin} `)}"`
const colors = [
['bgBlue.bold'],
['bgMagenta.bold'],
['bgGreen.bold'],
]
return `concurrently --prefix-colors "${colors.join(',')}" --prefix "[{name}]" --names "${names}" ${quotedScripts}`
}
8 changes: 3 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,15 @@
"dependencies": {
"arrify": "^1.0.1",
"chalk": "^1.1.3",
"commander": "^2.9.0",
"find-up": "^2.1.0",
"js-yaml": "^3.7.0",
"lodash": "^4.17.4",
"manage-path": "^2.0.0",
"omelette": "^0.3.1",
"prefix-matches": "^0.0.9",
"readline-sync": "^1.4.6",
"shell-escape": "^0.2.0",
"spawn-command-with-kill": "^1.0.0",
"type-detect": "^4.0.0"
"type-detect": "^4.0.0",
"yargs": "^6.6.0"
},
"devDependencies": {
"all-contributors-cli": "^3.1.0",
Expand All @@ -47,7 +45,7 @@
"cli-tester": "^2.0.0",
"codecov": "^1.0.1",
"commitizen": "^2.9.5",
"concurrently": "^3.1.0",
"concurrently": "^3.3.0",
"condition-node-version": "^1.3.0",
"cross-env": "^3.1.4",
"cz-conventional-changelog": "^1.2.0",
Expand Down
7 changes: 7 additions & 0 deletions src/__mocks__/console.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const console = require.requireActual('console')

module.exports = Object.assign({}, console, {
error: jest.fn(),
warn: jest.fn(),
info: jest.fn(),
})
Loading