GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — doc: improve `'hex'` Buffer decoding description and examples by gioragutt · Pull Request #41598 · nodejs/node

Conversation

gioragutt commented Jan 19, 2022 •

edited by aduh95

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Fixes: #41594

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with thumbs up emoji

All reactions

👍 1 reaction


          doc: add missing word in 'hex' Buffer decoding description

ccd7221

fixes nodejs#41594

nodejs-github-bot added buffer

Issues and PRs related to the buffer subsystem.

doc Issues and PRs related to Node.js documentation. labels

Jan 19, 2022

benjamingr approved these changes

Jan 19, 2022

View reviewed changes

benjamingr left a comment

Copy link

Copy Markdown

Member

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.

Please run the linter 🙏 ( the word 'hexadecimal' needs to drop a line)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 reacted with thumbs up emoji

All reactions

👍 1 reaction

aduh95 approved these changes

Jan 19, 2022

View reviewed changes

aduh95 reviewed

Jan 19, 2022

View reviewed changes

Comment thread

doc/api/buffer.md Resolved

Uh oh!

There was an error while loading. Please reload this page.


          fix max line width lint issue

7c9c04b

aduh95 added author ready

PRs with CI started, the required approvals, and no outstanding review comments.

commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels

Jan 19, 2022

tniessen reviewed

Jan 19, 2022

View reviewed changes

Comment thread

doc/api/buffer.md Outdated

		may occur when decoding strings that do exclusively contain valid hexadecimal
		characters. See below for an example.
		may occur when decoding strings that do not exclusively contain valid
		hexadecimal characters. See below for an example.

tniessen Jan 19, 2022

Copy link

Copy Markdown

Member

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.

Unfortunately, it doesn't matter much if the strings exclusively contain hexadecimal characters. In fact, it might be more surprising that Buffer.from truncates data even when it only contains hex digits:

// only hex, still truncated :(
Buffer.from('000', 'hex').toString('hex') === '00'

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gioragutt reacted with confused emoji

All reactions

😕 1 reaction

gioragutt Jan 19, 2022

Copy link

Copy Markdown

Contributor Author

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.

Is it because it reads 2 characters at a time?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

gioragutt Jan 19, 2022

Copy link

Copy Markdown

Contributor Author

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.

Nvm, read the docs, which state:

Buffer.from('1a7g', 'hex');
// Prints <Buffer 1a>, data truncated when data ends in single digit ('7').

So the last '0' is truncated because it's a single character, and the hex decoder expects pairs of characters.
So since the third '0' does not have a pairing character, it is truncated.

Perhaps the description should state that? You know, not only in the code example?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

aduh95 removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label

Jan 19, 2022

tniessen reviewed

Jan 19, 2022

View reviewed changes

Comment thread

doc/api/buffer.md Outdated

Comment on lines +178 to +179

		may occur when decoding strings that do not exclusively contain valid
		hexadecimal characters. See below for an example.

tniessen Jan 19, 2022

Copy link

Copy Markdown

Member

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.

Suggested change

	may occur when decoding strings that do not exclusively contain valid
	hexadecimal characters. See below for an example.
	may occur when decoding strings that do not exclusively consist of an even
	number of hexadecimal characters. See below for an example.

Maybe something like this?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr reacted with thumbs up emoji

All reactions

👍 1 reaction

gioragutt Jan 19, 2022

Copy link

Copy Markdown

Contributor Author

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.

I think that this, plus another code example (something like 123 -> 12) would be very nice

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

gioragutt Jan 19, 2022

Copy link

Copy Markdown

Contributor Author

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.

@tniessen WDYT?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

tniessen Jan 19, 2022

Copy link

Copy Markdown

Member

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.

SGTM. These are unfortunate oddities that we need to maintain for backward compatibility :(

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gioragutt reacted with thumbs up emoji

All reactions

👍 1 reaction

gioragutt Jan 19, 2022

Copy link

Copy Markdown

Contributor Author

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.

I took a better look at the examples and figured that I could achieve the same thing by altering the strings used in the examples.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

gioragutt added 2 commits

January 19, 2022 20:52


          Update buffer.md

eb2fcf5


          Update buffer.md

02371aa

tniessen approved these changes

Jan 19, 2022

View reviewed changes

addaleax approved these changes

Jan 19, 2022

View reviewed changes

Mesteery approved these changes

Jan 19, 2022

View reviewed changes

Mesteery added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label

Jan 19, 2022

lpinca approved these changes

Jan 19, 2022

View reviewed changes

Trott reviewed

Jan 19, 2022

View reviewed changes

Comment thread

doc/api/buffer.md

Comment on lines 177 to +179

		* `'hex'`: Encode each byte as two hexadecimal characters. Data truncation
		may occur when decoding strings that do exclusively contain valid hexadecimal
		characters. See below for an example.
		may occur when decoding strings that do not exclusively consist of an even
		number of hexadecimal characters. See below for an example.

Trott Jan 19, 2022

Copy link

Copy Markdown

Member

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.

Optional nit-pick suggestion: The Microsoft style guide we follow suggests might instead of may in this context. But I wonder if it's actually will occur rather than might occur? All told:

Suggested change

	* `'hex'`: Encode each byte as two hexadecimal characters. Data truncation
	may occur when decoding strings that do exclusively contain valid hexadecimal
	characters. See below for an example.
	may occur when decoding strings that do not exclusively consist of an even
	number of hexadecimal characters. See below for an example.
	* `'hex'`: Encode each byte as two hexadecimal characters. Data is truncated
	when decoding strings that do not exclusively consist of an even
	number of hexadecimal characters. See below for an example.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Trott approved these changes

Jan 19, 2022

View reviewed changes

gioragutt changed the title ~~doc: add missing word in 'hex' Buffer decoding description~~ doc: improve 'hex' Buffer decoding description and examples

Jan 20, 2022

aduh95 merged commit dbe60a2 into nodejs:master

Jan 21, 2022

aduh95 commented Jan 21, 2022

Copy link

Copy Markdown

Contributor

Landed in dbe60a2

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gioragutt deleted the patch-1 branch

January 22, 2022 11:14

BethGriggs pushed a commit that referenced this pull request

Jan 25, 2022


          doc: improve 'hex' Buffer decoding description and examples

81e039f

PR-URL: #41598
Fixes: #41594
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>

ruyadorno mentioned this pull request

Feb 8, 2022

v17.5.0 release proposal #41897

Merged

danielleadams pushed a commit that referenced this pull request

Feb 28, 2022


          doc: improve 'hex' Buffer decoding description and examples

3c8cf92

PR-URL: #41598
Fixes: #41594
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>

danielleadams pushed a commit that referenced this pull request

Mar 2, 2022


          doc: improve 'hex' Buffer decoding description and examples

76e2635

PR-URL: #41598
Fixes: #41594
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>

danielleadams mentioned this pull request

Mar 3, 2022

v16.14.1 release proposal #42200

Merged

danielleadams pushed a commit that referenced this pull request

Mar 3, 2022


          doc: improve 'hex' Buffer decoding description and examples

09df231

PR-URL: #41598
Fixes: #41594
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>

danielleadams pushed a commit that referenced this pull request

Mar 14, 2022


          doc: improve 'hex' Buffer decoding description and examples

5f0a017

PR-URL: #41598
Fixes: #41594
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>

Reviewers

aduh95

aduh95 approved these changes

Trott

Trott approved these changes

addaleax

addaleax approved these changes

benjamingr

benjamingr approved these changes

lpinca

lpinca approved these changes

tniessen

tniessen approved these changes

+1 more reviewer

Mesteery

Mesteery approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

buffer

Issues and PRs related to the buffer subsystem.

commit-queue-squash

PRs the Commit Queue should land as one squashed commit.

doc

Issues and PRs related to Node.js documentation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Buffer hex encoding description seems to omit a word

Uh oh!

There was an error while loading. Please reload this page.

9 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

gioragutt commented Jan 19, 2022 • edited by aduh95 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benjamingr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aduh95 commented Jan 21, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

gioragutt commented Jan 19, 2022 •

edited by aduh95

Loading

Uh oh!

There was an error while loading. Please reload this page.