Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: document undocumented methods on buffer prototype #48041

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Xstoudi
Copy link
Contributor

@Xstoudi Xstoudi commented May 16, 2023

Fixes: #46467

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels May 16, 2023
@Xstoudi Xstoudi changed the title doc: document undocumented method on buffer prototype doc: document undocumented methods on buffer prototype May 16, 2023
@Xstoudi Xstoudi force-pushed the doc/buffer-undocumented-slice-write branch from c19dbc9 to 69433b7 Compare May 17, 2023 18:40
@Xstoudi Xstoudi force-pushed the doc/buffer-undocumented-slice-write branch from ab71c21 to 3145d10 Compare May 25, 2023 18:41
@targos
Copy link
Member

targos commented May 30, 2023

@nodejs/buffer

@Xstoudi
Copy link
Contributor Author

Xstoudi commented Jul 22, 2023

Any chance it get merged?

@targos
Copy link
Member

targos commented Jul 24, 2023

@nodejs/documentation

@trivikr trivikr added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 27, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 27, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/48041
✔  Done loading data for nodejs/node/pull/48041
----------------------------------- PR info ------------------------------------
Title      doc: document undocumented methods on buffer prototype (#48041)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     Xstoudi:doc/buffer-undocumented-slice-write -> nodejs:main
Labels     buffer, doc, author ready
Commits    7
 - doc: document undocumented method on buffer prototype
 - doc: lint markdown
 - doc: remove trailing spaces
 - doc: forgot two trailing spaces
 - fix: remove trailing spaces
 - doc: add added date meta
 - doc: add missing console.log(bytesWritten)
Committers 2
 - Xavier Stouder 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/48041
Fixes: https://github.com/nodejs/node/issues/46467
Reviewed-By: James M Snell 
Reviewed-By: Trivikram Kamat 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/48041
Fixes: https://github.com/nodejs/node/issues/46467
Reviewed-By: James M Snell 
Reviewed-By: Trivikram Kamat 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - doc: add missing console.log(bytesWritten)
   ℹ  This PR was created on Tue, 16 May 2023 22:18:43 GMT
   ✔  Approvals: 2
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/48041#pullrequestreview-1449803511
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/48041#pullrequestreview-1549147920
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/5678981877

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jul 27, 2023
@trivikr trivikr added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jul 29, 2023
**Default**: `buf.length`.
* Returns: {string}

Decodes buf to a string according ASCII character encoding. `start` and
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Decodes buf to a string according ASCII character encoding. `start` and
Decodes `buf` to a string according ASCII character encoding. `start` and

**Default**: `buf.length`.
* Returns: {string}

Decodes buf to a string according to base64 character encoding. `start` and
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Decodes buf to a string according to base64 character encoding. `start` and
Decodes `buf` to a string according to base64 character encoding. `start` and

**Default**: `buf.length`.
* Returns: {string}

Decodes buf to a string according to base64url character encoding. `start`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Decodes buf to a string according to base64url character encoding. `start`
Decodes `buf` to a string according to base64url character encoding. `start`

**Default**: `buf.length`.
* Returns: {string}

Decodes buf to a string according to hex character encoding. `start` and
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Decodes buf to a string according to hex character encoding. `start` and
Decodes `buf` to a string according to hex character encoding. `start` and

**Default**: `buf.length`.
* Returns: {string}

Decodes buf to a string according to latin1 character encoding. `start` and
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Decodes buf to a string according to latin1 character encoding. `start` and
Decodes `buf` to a string according to latin1 character encoding. `start` and


### `buf.asciiWrite(string[, offset[, length]])`

* `string` {string} String to write to `buf`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the string needs to be ASCII? Or what happens if it contains non-ASCII chars?

Comment on lines +3783 to +3786
Writes `string` to `buf` at `offset` according to the base64 character encoding
and returns the number of bytes written. If `buf` did not contain enough space
to fit the entire string, only part of `string` will be written. However,
partially encoded characters will not be written.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I'd know what to expect if the input string contains non-ASCII chars, does it first convert to UTF-8 or some other encoding then convert it to base64?

Comment on lines +3821 to +3824
Writes `string` to `buf` at `offset` according to the base64url character
encoding and returns the number of bytes written. If `buf` did not contain
enough space to fit the entire string, only part of `string` will be written.
However, partially encoded characters will not be written.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here

Comment on lines +3859 to +3862
Writes `string` to `buf` at `offset` according to the hex character encoding
and returns the number of bytes written. If `buf` did not contain enough space
to fit the entire string, only part of `string` will be written. However,
partially encoded characters will not be written.
Copy link
Contributor

Choose a reason for hiding this comment

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

And here

Comment on lines +3908 to +3910
const bytesWritten = buf.latin1Write('buffer');
console.log(bytesWritten);
// Prints: 2
Copy link
Contributor

Choose a reason for hiding this comment

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

How does 'buffer' fit in a 2-byte representation? 🤔

@jasnell jasnell removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undocumented buffer.***Slice() methods
7 participants