Skip to content

doc: general improvements to string_decoder.md copy#6940

Closed
jasnell wants to merge 4 commits intonodejs:masterfrom
jasnell:doc-string-decoder-copy
Closed

doc: general improvements to string_decoder.md copy#6940
jasnell wants to merge 4 commits intonodejs:masterfrom
jasnell:doc-string-decoder-copy

Conversation

@jasnell
Copy link
Copy Markdown
Member

@jasnell jasnell commented May 23, 2016

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

doc (string_decoder)

Description of change

General improvements to string_decoder.md

@nodejs/documentation

@jasnell jasnell added doc Issues and PRs related to the documentations. string_decoder Issues and PRs related to the string_decoder subsystem. labels May 23, 2016
Comment thread doc/api/string_decoder.md Outdated
## Class: StringDecoder
When a `Buffer` instance is written to the `StringDecoder` instance, an
internal buffer is used to ensure that the decoded string does not contain
any partial multibyte-characters. These are held in the buffer until the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tiny nit: multibyte characters (and I’d maybe call them incomplete instead of partial but that’s certainly a question of personal taste)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

@jasnell
Copy link
Copy Markdown
Member Author

jasnell commented May 23, 2016

nits addressed!

@addaleax
Copy link
Copy Markdown
Member

LGTM!

Comment thread doc/api/string_decoder.md Outdated
Creates a new `StringDecoder` instance.

### decoder.end()
### stringDecoder.end()
Copy link
Copy Markdown
Contributor

@mscdex mscdex May 24, 2016

Choose a reason for hiding this comment

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

Actually end() can accept a Buffer just like write(). In that case, .write(buffer) is called first before adding any replacement characters.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point! I'd forgotten about that and it wasn't previously documented.

@jasnell
Copy link
Copy Markdown
Member Author

jasnell commented May 24, 2016

@mscdex ... updated! I updated the example also to show a Buffer being passed to end()

Comment thread doc/api/string_decoder.md Outdated
Creates a new `StringDecoder` instance.

### stringDecoder.end()
### stringDecoder.end(buffer)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/buffer/[buffer]/

@jasnell
Copy link
Copy Markdown
Member Author

jasnell commented May 24, 2016

@mscdex ... done!

@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented May 24, 2016

LGTM

jasnell added a commit that referenced this pull request May 25, 2016
PR-URL: #6940
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Brian White <[email protected]>
@jasnell
Copy link
Copy Markdown
Member Author

jasnell commented May 25, 2016

Landed in 9140b97. Thank you!

@jasnell jasnell closed this May 25, 2016
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 30, 2016
PR-URL: nodejs#6940
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Brian White <[email protected]>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
PR-URL: #6940
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Brian White <[email protected]>
@MylesBorins
Copy link
Copy Markdown
Contributor

adding don't land label. Please feel free to backport @jasnell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Issues and PRs related to the documentations. string_decoder Issues and PRs related to the string_decoder subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants