This repository has been archived by the owner on Apr 22, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Fix child process docs #8639
Closed
Closed
Fix child process docs #8639
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
6f898d4
doc: remove wrong spawn error checking example
sam-github e206947
doc: remove wrong note about spawn env
sam-github b4098b9
doc: spawn cwd default is current working dir
sam-github 6a816e8
doc: hyperlink and re-order spawn documentation
sam-github cd44cb0
doc: add the missing ChildProcess.stdio property
sam-github ce64ece
doc: ending child_process.stdin does not terminate
sam-github 6154652
doc: encoding option for fork does not exist
sam-github File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you:
#### Sharing non-stdio file descriptors
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't make sense, you'd have:
That's a heading saying "non-studio", followed by a sentence documenting _stdio_.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the whole paragraph is basically just a factoring out (not by me) of common prose for options 1, 3, 4, and 5. Without this info, you don't know what the pipes create in 1., for example, look like in the child, or how to use them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair. The reason I was looking for that reorganization is that the new paragraph pushes down the example snippet, while leading into it with a sentence describing a feature that is not often utilized.
It definitely struck me as something that needs a code example, and if I were coming to the docs fresh, I could definitely see myself losing a few moments reconciling that sentence with the succeeding example code. However, you're right, it doesn't make sense to immediately launch a paragraph about non-stdio fd's with a sentence describing fds.
If it's the case that sharing non-stdio fds are not widely supported, then we can create the aforementioned level-4 subsection describing them and how they're not available on windows.