doc: improve child_process.markdown copy #4383
Conversation
jasnell
added
child_process
| assert.equal(child.stdio[2], null); | ||
| assert.equal(child.stdio[2], child.stderr); | ||
| Pipes for `stdin`, `stdout` and `stderr` are established between the parent | ||
| Node.js process and the created child. It is possible to stream data through |
There was a problem hiding this comment.
Maybe replace "created" with "spawned" here.
There was a problem hiding this comment.
They are only established when spawn is called with pipe as an option, or fork with silent: true. Unless I misunderstand the diff, this statement seems much more sweeping than is accurate.
There was a problem hiding this comment.
I will clarify that this is the default behavior
|
Left a bunch of comments, but generally LGTM |
| and asynchronous alternatives to `child_process.spawn()`, each of which are | ||
| documented fully [below][]: | ||
|
|
||
| * `child_process.exec()`: spawns a shell and runs a command within that shell. |
There was a problem hiding this comment.
... "returning the stdout and stderr when the command completes". <--- that last is the very important distinction between the exec* and the spawn/fork.
| be launched using `child_process.execFile()` (or even `child_process.spawn()`). | ||
| When running on Windows, `.bat` and `.cmd` files can only be invoked using | ||
| either `child_process.exec()` or by spawning `cmd.exe` and passing the `.bat` | ||
| or `.cmd` file as an argument. |
There was a problem hiding this comment.
(which is what child_process.exec() does). <-- worth mentioning? to demystify?
|
@jasnell Looks like you addressed all my comments, I remembered a couple other useful tidbits of info, though. |
|
@sam-github ... ok, pushed another set of edits. |
|
@nodejs/collaborators ... can I ask for some additional review and sign off on this? Thank you! |
General improvements to child_process.markdown
jasnell
force-pushed
the
doc-child-process-improvements
branch
from
3631046
to
f53b65d
Compare
|
LGTM |
| sending messages between parent and child. | ||
| * `child_process.execSync()`: a synchronous version of | ||
| `child_process.exec()` that *will* block the Node.js event loop. | ||
| * `child_process.execFileSync()`: a synchronous version of |
There was a problem hiding this comment.
Does it make sense to list the Sync versions right below their asynchronous counterpart?
There was a problem hiding this comment.
I'd prefer to keep them grouped by async vs sync
General improvements to child_process.markdown PR-URL: #4383 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
Landed in 7d5c1b5. Can make additional improvements if necessary in a separate PR |
|
|
||
| const exec = require('child_process').execFile; |
There was a problem hiding this comment.
Sadly, this code example doesn't work even on Unix-like platforms. Since there's no shell wrapper,
filemust be an absolute path to executable.argsmust contain all the command line arguments forfile.- All paths within
argsmust be absolute as well. - Stdio redirection is not possible.
Here's an alternative I whipped up that should work on all Unix-like platforms. As always, feel free to use none, some, or all of it.
const execFile = require('child_process').execFile;
const child = execFile('/bin/cat', ['/etc/paths'], (error, stdout, stderr) => {
if (error) {
throw error;
}
console.log(stdout);
});
|
@ryansobol ... can I ask you for a quick PR to update the example? :-) Thank you! |
General improvements to child_process.markdown PR-URL: nodejs#4383 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
@jasnell this is not merging cleanly, can you backport this please. |
|
@thealphanerd ..yes, I'll work on backporting all of these kind of doc changes. They may not all make it into v4.2.5 but I'll work on them |
|
@thealphanerd ... I'll be working on porting this to LTS early next week. |
General improvements to child_process.markdown PR-URL: nodejs#4383 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
General improvements to child_process.markdown PR-URL: #4383 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
General improvements to child_process.markdown PR-URL: #4383 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
General improvements to child_process.markdown PR-URL: #4383 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
General improvements to child_process.markdown PR-URL: nodejs#4383 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
General improvements to child_process.markdown