Better stdio option handling - fixes #24

* Better stdio option handling - fixes #24 

* Based on work started in #80

* tweak docs

* add fd test

* add number tests and fix the bugs

index.js

@@ -9,6 +9,7 @@ const _getStream = require('get-stream');
 const pFinally = require('p-finally');
 const onExit = require('signal-exit');
 const errname = require('./lib/errname');
+const stdio = require('./lib/stdio');
 
 const TEN_MEGABYTES = 1000 * 1000 * 10;
 
@@ -37,6 +38,8 @@ function handleArgs(cmd, args, opts) {
 		cleanup: true
 	}, parsed.options);
 
+	opts.stdio = stdio(opts);
+
 	if (opts.preferLocal) {
 		opts.env = npmRunPath.env(opts);
 	}
@@ -206,7 +209,20 @@ module.exports = (cmd, args, opts) => {
 
 		if (err || code !== 0 || signal !== null) {
 			if (!err) {
-				const output = parsed.opts.stdio === 'inherit' ? '' : `\n${stderr}${stdout}`;
+				let output = '';
+
+				if (Array.isArray(parsed.opts.stdio)) {
+					if (parsed.opts.stdio[2] !== 'inherit') {
+						output += output.length > 0 ? stderr : `\n${stderr}`;
+					}
+
+					if (parsed.opts.stdio[1] !== 'inherit') {
+						output += `\n${stdout}`;
+					}
+				} else if (parsed.opts.stdio !== 'inherit') {
+					output = `\n${stderr}${stdout}`;
+				}
+
 				err = new Error(`Command failed: ${joinedCmd}${output}`);
 				err.code = code < 0 ? errname(code) : code;
 			}

lib/stdio.js

@@ -0,0 +1,41 @@
+'use strict';
+const alias = ['stdin', 'stdout', 'stderr'];
+
+const hasAlias = opts => alias.some(x => Boolean(opts[x]));
+
+module.exports = opts => {
+	if (!opts) {
+		return null;
+	}
+
+	if (opts.stdio && hasAlias(opts)) {
+		throw new Error(`It's not possible to provide \`stdio\` in combination with one of ${alias.map(x => `\`${x}\``).join(', ')}`);
+	}
+
+	if (typeof opts.stdio === 'string') {
+		return opts.stdio;
+	}
+
+	const stdio = opts.stdio || [];
+
+	if (!Array.isArray(stdio)) {
+		throw new TypeError(`Expected \`stdio\` to be of type \`string\` or \`Array\`, got \`${typeof stdio}\``);
+	}
+
+	const result = [];
+	const len = Math.max(stdio.length, alias.length);
+
+	for (let i = 0; i < len; i++) {
+		let value = null;
+
+		if (stdio[i] !== undefined) {
+			value = stdio[i];
+		} else if (opts[alias[i]] !== undefined) {
+			value = opts[alias[i]];
+		}
+
+		result[i] = value;
+	}
+
+	return result;
+};

package.json

@@ -59,6 +59,7 @@
     "delay": "^1.3.1",
     "is-running": "^2.0.0",
     "nyc": "^10.0.0",
+    "tempfile": "^1.1.1",
     "xo": "*"
   },
   "nyc": {

readme.md

@@ -146,6 +146,27 @@ Default: `true`
 
 Keep track of the spawned process and `kill` it when the parent process exits.
 
+#### stdin
+
+Type: `string` `number` `Stream` `undefined` `null`<br>
+Default: `pipe`
+
+Same options as [`stdio`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio).
+
+#### stdout
+
+Type: `string` `number` `Stream` `undefined` `null`<br>
+Default: `pipe`
+
+Same options as [`stdio`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio).
+
+#### stderr
+
+Type: `string` `number` `Stream` `undefined` `null`<br>
+Default: `pipe`
+
+Same options as [`stdio`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio).
+
 
 ## Tips
 

test.js

@@ -1,10 +1,12 @@
 import path from 'path';
+import fs from 'fs';
 import stream from 'stream';
 import childProcess from 'child_process';
 import test from 'ava';
 import getStream from 'get-stream';
 import isRunning from 'is-running';
 import delay from 'delay';
+import tempfile from 'tempfile';
 import m from './';
 
 process.env.PATH = path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH;
@@ -45,7 +47,41 @@ test('include stdout and stderr in errors for improved debugging', async t => {
 
 test('do not include in errors when `stdio` is set to `inherit`', async t => {
 	const err = await t.throws(m('fixtures/error-message.js', {stdio: 'inherit'}));
-	t.notRegex(err.message, /\\n/);
+	t.notRegex(err.message, /\n/);
+});
+
+test('do not include `stderr` and `stdout` in errors when set to `inherit`', async t => {
+	const err = await t.throws(m('fixtures/error-message.js', {stdout: 'inherit', stderr: 'inherit'}));
+	t.notRegex(err.message, /\n/);
+});
+
+test('do not include `stderr` and `stdout` in errors when `stdio` is set to `inherit`', async t => {
+	const err = await t.throws(m('fixtures/error-message.js', {stdio: [null, 'inherit', 'inherit']}));
+	t.notRegex(err.message, /\n/);
+});
+
+test('do not include `stdout` in errors when set to `inherit`', async t => {
+	const err = await t.throws(m('fixtures/error-message.js', {stdout: 'inherit'}));
+	t.notRegex(err.message, /stdout/);
+	t.regex(err.message, /stderr/);
+});
+
+test('do not include `stderr` in errors when set to `inherit`', async t => {
+	const err = await t.throws(m('fixtures/error-message.js', {stderr: 'inherit'}));
+	t.regex(err.message, /stdout/);
+	t.notRegex(err.message, /stderr/);
+});
+
+test('pass `stdout` to a file descriptor', async t => {
+	const file = tempfile('.txt');
+	await m('fixtures/noop', ['foo bar'], {stdout: fs.openSync(file, 'w')});
+	t.is(fs.readFileSync(file, 'utf8'), 'foo bar\n');
+});
+
+test('pass `stderr` to a file descriptor', async t => {
+	const file = tempfile('.txt');
+	await m('fixtures/noop-err', ['foo bar'], {stderr: fs.openSync(file, 'w')});
+	t.is(fs.readFileSync(file, 'utf8'), 'foo bar\n');
 });
 
 test('execa.shell()', async t => {

test/stdio.js

@@ -0,0 +1,50 @@
+import util from 'util';
+import test from 'ava';
+import stdio from '../lib/stdio';
+
+util.inspect.styles.name = 'magenta';
+
+function macro(t, input, expected) {
+	if (expected instanceof Error) {
+		t.throws(() => stdio(input), expected.message);
+		return;
+	}
+
+	const result = stdio(input);
+
+	if (typeof expected === 'object' && expected !== null) {
+		t.deepEqual(result, expected);
+	} else {
+		t.is(result, expected);
+	}
+}
+
+macro.title = (providedTitle, input) => providedTitle || util.inspect(input, {colors: true});
+
+test(macro, undefined, null);
+test(macro, null, null);
+
+test(macro, {stdio: 'inherit'}, 'inherit');
+test(macro, {stdio: 'pipe'}, 'pipe');
+test(macro, {stdio: 'ignore'}, 'ignore');
+test(macro, {stdio: [0, 1, 2]}, [0, 1, 2]);
+
+test(macro, {}, [null, null, null]);
+test(macro, {stdio: []}, [null, null, null]);
+test(macro, {stdin: 'pipe'}, ['pipe', null, null]);
+test(macro, {stdout: 'ignore'}, [null, 'ignore', null]);
+test(macro, {stderr: 'inherit'}, [null, null, 'inherit']);
+test(macro, {stdin: 'pipe', stdout: 'ignore', stderr: 'inherit'}, ['pipe', 'ignore', 'inherit']);
+test(macro, {stdin: 'pipe', stdout: 'ignore'}, ['pipe', 'ignore', null]);
+test(macro, {stdin: 'pipe', stderr: 'inherit'}, ['pipe', null, 'inherit']);
+test(macro, {stdout: 'ignore', stderr: 'inherit'}, [null, 'ignore', 'inherit']);
+test(macro, {stdin: 0, stdout: 1, stderr: 2}, [0, 1, 2]);
+test(macro, {stdin: 0, stdout: 1}, [0, 1, null]);
+test(macro, {stdin: 0, stderr: 2}, [0, null, 2]);
+test(macro, {stdout: 1, stderr: 2}, [null, 1, 2]);
+
+test(macro, {stdio: {foo: 'bar'}}, new TypeError('Expected `stdio` to be of type `string` or `Array`, got `object`'));
+
+test(macro, {stdin: 'inherit', stdio: 'pipe'}, new Error('It\'s not possible to provide `stdio` in combination with one of `stdin`, `stdout`, `stderr`'));
+test(macro, {stdin: 'inherit', stdio: ['pipe']}, new Error('It\'s not possible to provide `stdio` in combination with one of `stdin`, `stdout`, `stderr`'));
+test(macro, {stdin: 'inherit', stdio: [undefined, 'pipe']}, new Error('It\'s not possible to provide `stdio` in combination with one of `stdin`, `stdout`, `stderr`'));