Skip to content

Commit

Permalink
fix(cli): improve handling of forwarding args/flags
Browse files Browse the repository at this point in the history
**What**: This migrates us from `commander` and `omelette` to `yargs`
and in the process changes how we handle forwarding of arguments and
flags. We no longer just arbitrarily forward arguments to every script.
Also instead of comma separated scripts, they're simply separated by a
space. If you want to pass args/flags to a script, then you put it in
quotes.

**Why**: Makes this more predicatble. Also `yargs` is more powerful in
general anyway.

**How**: Magic... Will comment inline...
  • Loading branch information
Kent C. Dodds committed Feb 13, 2017
1 parent 8c04785 commit faf5b66
Show file tree
Hide file tree
Showing 18 changed files with 328 additions and 427 deletions.
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:
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 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
24 changes: 0 additions & 24 deletions cli-test/__snapshots__/cli.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -31,27 +31,3 @@ 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\"
",
}
`;
5 changes: 0 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',
() => snapshot('-c ./package-scripts-with-default.js'),
Expand Down
2 changes: 1 addition & 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 Down
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
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
37 changes: 0 additions & 37 deletions src/bin-utils/autocomplete/index.js

This file was deleted.

67 changes: 0 additions & 67 deletions src/bin-utils/autocomplete/index.test.js

This file was deleted.

Loading

0 comments on commit faf5b66

Please sign in to comment.