Skip to content

Conversation

@sartrey
Copy link
Contributor

@sartreysartrey commented Jun 22, 2016

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)
Description of change

add words in child_process docs about invalid Buffer encoding leading to a default buffer output

@nodejs-github-botnodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Jun 22, 2016
@sartreysartrey changed the title fix doc about child_process exec & execFiledoc: fix exec stdout & stderr default type in child_processJun 22, 2016
@sartrey
Copy link
ContributorAuthor

#6666

the output as UTF-8 and pass strings to the callback. The `encoding` option
can be used to specify the character encoding used to decode the stdout and
stderr output. If `encoding` is `'buffer'`, `Buffer` objects will be passed to
stderr output. If `encoding` is `'buffer'` (or not a valid Buffer encoding), `Buffer` objects will be passed to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should say something like:

If encoding is 'buffer', or an unrecognized character encoding, Buffer objects will be passed to

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please wrap lines at 80 characters.

@sartrey
Copy link
ContributorAuthor

@cjihrig I will edit these lines.

can be used to specify the character encoding used to decode the stdout and
stderr output. If `encoding` is `'buffer'`, `Buffer` objects will be passed to
the callback instead.
stderr output. If `encoding` is `'buffer'`, or an unrecognized character encoding,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a quick check, it seems like these lines are still > 80 characters.

@sartrey
Copy link
ContributorAuthor

@cjihrig
Yeah, these lines are still > 80 characters.
However, I found there are some long lines before my change.
I try to find balance between line wrap & easier to read.
Please tell me if I should keep wrap at accurate 80 characters.

Sorry for my poor English, here is my true meaning in Chinese

我在文档的前半部分看到一些行也超过了80字符,
看上去可能是要保持一行意义完整。
所以,我也尝试在一行里取得某种平衡。
改动后的行大约80字符左右,可能有一点超过。

如果我必须精确遵循80字符,请告诉我,我会修改的。

@cjihrig
Copy link
Contributor

Please wrap them at 80 characters or less. There are some other longer lines, but they shouldn't be that way (no need to change the other ones in this PR though).

@sartrey
Copy link
ContributorAuthor

@cjihrig Please review this new commit.

@cjihrig
Copy link
Contributor

LGTM

cjihrig pushed a commit that referenced this pull request Jun 27, 2016
Clarify how the encoding option interacts with the data type of child process stdout and stderr. Fixes: #6666 PR-URL: #7361 Reviewed-By: Colin Ihrig <[email protected]>
@cjihrig
Copy link
Contributor

Thanks! Landed in 926707f.

@cjihrigcjihrig closed this Jun 27, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
Clarify how the encoding option interacts with the data type of child process stdout and stderr. Fixes: #6666 PR-URL: #7361 Reviewed-By: Colin Ihrig <[email protected]>
@Fishrock123Fishrock123 mentioned this pull request Jul 5, 2016
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
Clarify how the encoding option interacts with the data type of child process stdout and stderr. Fixes: #6666 PR-URL: #7361 Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Clarify how the encoding option interacts with the data type of child process stdout and stderr. Fixes: #6666 PR-URL: #7361 Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Clarify how the encoding option interacts with the data type of child process stdout and stderr. Fixes: #6666 PR-URL: #7361 Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Clarify how the encoding option interacts with the data type of child process stdout and stderr. Fixes: #6666 PR-URL: #7361 Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jul 12, 2016
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Clarify how the encoding option interacts with the data type of child process stdout and stderr. Fixes: #6666 PR-URL: #7361 Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Clarify how the encoding option interacts with the data type of child process stdout and stderr. Fixes: #6666 PR-URL: #7361 Reviewed-By: Colin Ihrig <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

child_processIssues and PRs related to the child_process subsystem.docIssues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@sartrey@cjihrig@MylesBorins@nodejs-github-bot