Skip to content

doc: general improvements to string_decoder.md copy - #6940

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

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

Conversation

@jasnell

@jasnell jasnell commented May 23, 2016

Copy link
Copy Markdown
Member
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

jasnell commented May 23, 2016

Copy link
Copy Markdown
Member Author

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()

@mscdex mscdex May 24, 2016

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.

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

jasnell commented May 24, 2016

Copy link
Copy Markdown
Member Author

@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

jasnell commented May 24, 2016

Copy link
Copy Markdown
Member Author

@mscdex ... done!

@mscdex

mscdex commented May 24, 2016

Copy link
Copy Markdown
Contributor

LGTM

jasnell added a commit that referenced this pull request May 25, 2016
PR-URL: #6940
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Brian White <mscdex@mscdex.net>
@jasnell

jasnell commented May 25, 2016

Copy link
Copy Markdown
Member Author

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 <anna@addaleax.net>
Reviewed-By: Brian White <mscdex@mscdex.net>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
PR-URL: #6940
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Brian White <mscdex@mscdex.net>
@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