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

Suggestion: use child_process.exec instead of spawn in child_process helper task #58

Closed
sean-lynch opened this issue Mar 6, 2012 · 4 comments

Comments

@sean-lynch
Copy link

I've been migrating our custom tasks to use the built-in child_process helper task rather than our custom equivalent. I realized that the built in child_process uses spawn and two custom arrays to buffer output when exec provides this built in.

On top of that, .exec operates more closely to actual command line calls which is what I imagine child_process is typically used to emulate. For example, the command handlebars templates/*.html --output=templates.js works fine on the command line and in child_process.exec because they both expand the * wildcard. It fails when using spawn because spawn does not expand the wildcard.

The following is an implementation of the child_process helper using exec while maintaining the exact same function definition: https://gist.github.com/1989146

@cowboy
Copy link
Member

cowboy commented Mar 9, 2012

Joining opts.args on space is a little naive, as arguments could contain spaces. Trying to quote them could be a huge mess.

@sean-lynch
Copy link
Author

Agreed on the naivety, that was the trade-off for maintaining a backwards compatible argument with the existing implementation. I guess from my perspective, if I had to put quotes around the argument in the command line, I'd expect to have to do the same when passing those values to this helper.

@cowboy
Copy link
Member

cowboy commented Mar 10, 2012

The built-in grunt file.expand method handles wildcard globbing, so you don't need to rely on the shell to do that. Not to mention that file globbing rules might be different on Windows, which could potentially cause portability issues.

I recommend using the built-in child_process helper (note, I just rewrote it today) along with file.expand to handle these scenarios. Also, you will probably have more control if you use handlebars programmatically via its JavaScript API instead of invoking it as a child process.

@cowboy cowboy closed this as completed Mar 10, 2012
@sean-lynch
Copy link
Author

Cool, thanks very much for the pointer to file.expand, that accomplishes what I was looking for. I've trashed my implementation and I'll keep an eye out for the child_process to util move in the next release. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants