This reworks the plugin API such that:
- Unable to register a command with unknown wrap-options
- `TypeError` raised for wrap-option type mistakes
- Remove the `overWrite` option (it's unused, probably safest to not
expose for now)
- `cmdOptions` defaults to `null` instead of `false` for type
consistency (no change to default behavior)
- Move `pipeMethods` logic into `_register`, since it makes more sense
there
This is not expected to have any effect on existing plugins.
The `paramsFile` is obsolete now that we use `execFileSync()` for our
internal implementation. Instead, we pass parameters to the child
process directly as a single commandline parameter to reduce file I/O.
Issue #782
This parameter isn't needed, we can easily rely on exit code status for this.
Eliminating the parameter reduces file IO, code complexity, and removes a busy
loop.
This also removes some legacy code related to streams.
Issue #782
Eslint rules can be configured either using words or number values:
* "off" or 0
* "warn" or 1
* "error" or 2
This switches our config to use the string values instead of the number
values, since the number values are too cryptic.
No change to our actual settings.
This adds the special option string `--`, which means "no options". This can be
passed if the first parameter looks like an option (starts with a `-` followed
by 1+ letters).
Fixes#778
This uses `child_process.execFileSync` instead of `execSync` to launch the child
process. This further reduces the attack surface, removing a possible point for
command injection in the ShellJS implementation.
This does not affect backwards compatibility for the `shell.exec` API (the
behavior is determined by the call to `child_process.exec` within
`src/exec-child.js`).
Issue #782
This PR refactors `shell.exec()` by putting its child process in a separate code
file. This also slightly cleans up dead code.
There's more potential to clean this up (e.g. exit status), but this is a good
enough start.
Issue #782
Adds two new methods to src/common.js, common.statFollowLinks and common.statNoFollowLinks, which wrap fs.statSync and fs.lstatSync, respectively. This change is meant to improve readability and clarify intent.
* Add common.statFollowLinks and common.statNoFollowLinks
* Replace fs.statSync and fs.lstatSync in source files
This sets 2 AVA options:
* `serial`: same behavior as the CLI flag, which this replaces
* `powerAssert`: if an assert fails, it will inspect all objects involved in
the failed assert. This causes readability issues if `t.falsy(shell.error())`
breaks, since it inspects all of `shell` (which is way too large).
AVA options are a little easier to manage than CLI options (we only update one
place instead of 2 in `package.json`).
* Added `-q` (quiet) option to `push`, `popd`, `dirs` functions.
* Added unit tests for pushd/popd quiet mode.
* Added tests for `pushd` and `popd` with quiet mode off.
* Updated docs for `pushd` and `popd` functions.
* Moved preliminary `pushd` commands for `popd` tests before disabling of silent flag.
This updates tests for `AVA` 19.0.0+.
`AVA` 0.18.0 has a breaking change which changes the current working directory
for tests. As a result, we need to change 'resources' -> 'test/resources' (and
similar path changes).
`AVA` 0.19.0 has a breaking change that all tests must run at least one assert.
This breaking change was already resolved by #746, so no change was necessary in
this PR.
This updates to `AVA` 0.21.0, since there are no other breaking changes.
This adds `skipOnWin` and `skipOnUnix` to help us manage our platform-dependent
tests. These methods give a nice warning message when we skip tests. We may also
consider adding warnings when running platform-dependent tests.
Part of the motivation for this is if we ever update to AVA v0.19. This version
requires at least one assertion per test case. While this could be disabled with
an AVA setting, we instead benefit from warnings for any case when we
unintentionally skip assertions.
This adds chalk as a dev dependency to enable colored messages.
* test(exec): add tests for coverage
No logic change.
This adds one test to cover some missing lines, and adds some `istanbul ignore`
directives.
I see 100% line coverage for `src/exec.js` when running:
```sh
$ nyc --reporter=text --reporter=lcov ava --serial test/exec.js`
```
Fixes#742
* Fix lint
This adds a test for `head()` on the right-hand side of a pipe. This also
removes the try-catch surrounding `fs.openSync()`, because it was unreachable
code. `fs.existsSync()` guarantees that the file exists, and `fs.openSync()`
only throws if the file does not exist, according to official documentation.
Fixes#671
* Add stdout/stderr test mocks
* Mock stdout/stderr during echo tests
* Fix lint issues
* Use 'use strict'
* Re-implement mocks as a prototype
* Implement mocks as a single-instance
* Remove redundant test
* Create mocked stdout/stderr.write methods once
* Add newline to output of echo (#557)
* Add newline to output of echo
* Add test
* Throw an error if the options string does not start with '-' (#615)
* Throw an error if the options string does not start with '-'
* Add test
* Change message grammar
* Add -n option to echo
* Fix null argument issue
* Add -n tests
* Add documentation
* Add -en escaped character test
* Add function to parse options for echo
* Use parseOptions to parse echo options
* Simplify control flow
* parseOptions throws now
* Allow null to be echoed
* Prevent echo stderr on unrecognized option
* Add test to check stderr of returned value
* Use consistent variable name
* Change test message, leave TODO about console output
* fix(mkdir): improve error handling around files
In particular, this fixes:
- if we try to overwrite a file with a mkdir
- if we try to create a subdirectory of a file
- adds `continue: true` in spots where we missed it
Fixes#720
* Fixing tests on Windows
* fix(cat): do not cat directories
Fixes#707
* fix(head): do not let head() read directories
Also fixes a typo
* fix(sort): do not sort directories
Also fixes a typo
* fix(tail): do not let tail() read directories
Also fixes a typo
* fix(uniq): do not let uniq() read directories
We also had a test which called sort() instead of uniq(), so we never
actually tested the missing-file case. This fixes that as well.
This also throws an error for using a directory as output.
* fix(pipe): fix breakages with piped commands
* cp: add error to not overwrite recently created files #631
* cp: add tests for errors not overwrite recently created files #631
* mv: show error when overwriting recently created file #631
* mv: add tests for error on recently created files #631
* mv: test remove unnecessary steps #631
* rm -rf on a symlink to a dir deletes contents
* Fix comment typo
* Clarify comments
* Pass symlink directly to rmdirSync
* Skip rm -rf symlink test on windows
* refactor(parseOptions): better handle errors
For the case where `parseOptions()` is passed a string that does not
start with a hyphen, we should reject this string instead of returning
the same value. This has no change on any other test cases, and should
not affect any commands since we are careful about what input we pass to
`parseOptions()`
This also adjust how we were handling error cases in the function. We
were previously using two different calls to `common.error()`, one for
if `errorOptions` is passed, and one without. This hurts readability,
because it reads like "if we have errorOptions, then this is an error,
and if we don't then it's also an error," instead of the more
appropriate, "we reached an error case and should use errorOptions if
it's available, otherwise we should signal an error without using it."
We do not need to use `errorOptions` for the added call to `error()`,
since this is an error which indicates misuse, and should not be
recoverable.
This adds a test case as well, for a line which was not previously
covered in our tests.
Partial fix for #671
* Refactor parseOptions()
This validates input at the top of `parseOptions()` for type
correctness. This changes `typeof x === 'object'` to `x instanceOf
Object` to improve robustness. Now we can safely use `errorOptions`
knowing that if it has a truthy value, it will be an object, otherwise
we should use defaults instead.
The new tests ensure that we have 100% unit test coverage for this
function.
* Use exceptions, not common.error()s, for internal misuse
* Add common.isObject() helper function
The `isObject()` helper is used throughout common.js to reliably test if
variables are JavaScript objects.
