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

Improve usability of child process's stream interface for util.spawn #699

Closed

Conversation

jharding
Copy link

@jharding jharding commented Mar 1, 2013

Right now, if the command passed to util.spawn isn't found, undefined is returned and the done callback is invoked. This makes it impossible to rely entirely on the stream interface of the spawned child process. For example, I couldn't do the following:

var child = grunt.spawn('notacommand');

child.stdout.on('data', function(d) { grunt.log.write(d); });
child.stderr.on('data', function(d) { grunt.log.error(d); });

Instead, I'd have to do something like:

var child = grunt.spawn('notacommand', function(error, result, code) {
  code !== 0 && grunt.log.error(result.stderr);
});

child && child.stdout.on('data', function(d) { grunt.log.write(d); });
child && child.stderr.on('data', function(d) { grunt.log.error(d); });

This pull request adds support for the former example. The downside to this change is that the error message for when a command is not found is not as descriptive as it previously was. If this is a sticking point for this pull request, I'll address the regression.

One other note – I didn't test this out on Windows.

@shama
Copy link
Member

shama commented Mar 1, 2013

+1

1 similar comment
@sindresorhus
Copy link
Member

👍

@cowboy
Copy link
Member

cowboy commented Mar 12, 2013

I'm hesitant to make this change, due to the fact that it could break existing code in a great number of projects and plugins.

@jharding
Copy link
Author

So I guess the only "API" change I've made is that undefined no longer gets returned if node-which fails to find the command. Is that something you think a lot of projects depend on? I honestly haven no idea if it is or isn't.

@cowboy
Copy link
Member

cowboy commented Mar 12, 2013

Well, I'm not sure. If someone is doing if (child) { ... } then it will definitely break their code.

@vladikoff
Copy link
Member

Due to a breaking change we push it to 0.next, Tracking in gruntjs/grunt-next#2

@vladikoff vladikoff closed this Feb 26, 2014
@vladikoff vladikoff modified the milestones: 0.4.3, 0.4.5 Mar 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants