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: clarify the parameter byteOffset for lastIndexOf method of Buffer #38167

Closed

Conversation

arkerone
Copy link
Contributor

@arkerone arkerone commented Apr 9, 2021

I suggest to clarify the parameter byteOffset for the lastIndexOf method of Buffer.

Currently, the doc says :
"Where to begin searching in buf"

But in my opinion, it's not just. The parameter is more :
"the index of the last byte in buf to be considered as the beginning of a match."

What do you think about that ?

@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 Apr 9, 2021
@jasnell
Copy link
Member

jasnell commented Apr 28, 2021

/cc @Trott

@Trott
Copy link
Member

Trott commented May 1, 2021

I find the proposed wording harder to understand, but maybe that's just me. Do others think it is either more precise or easier to read? @nodejs/documentation

Comment on lines +1408 to +1409
* `byteOffset` {integer} The index of the last byte in `buf` to be considered
as the beginning of a match. If negative, then offset is calculated
Copy link
Contributor

@DerekNonGeneric DerekNonGeneric May 2, 2021

Choose a reason for hiding this comment

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

Suggested change
* `byteOffset` {integer} The index of the last byte in `buf` to be considered
as the beginning of a match. If negative, then offset is calculated
* `byteOffset` {integer} The last zero-based index (in bytes) of `buf` to be
considered as the beginning of a match. If negative, then offset is calculated

My 2¢, but given that Buffer is a subclass of Uint8Array, we may be able to use some of the language used by MDN if it's more suitable.

The Buffer class is a subclass of JavaScript's Uint8Array class and extends it with methods that cover additional use cases.

See: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/byteOffset

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find the proposed wording harder to understand, but maybe that's just me. Do others think it is either more precise or easier to read? @nodejs/documentation

I was inspired by the documentation concerning the lastIndexOf method of string (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/lastIndexOf)) but we can also draw inspiration from the documentation of TypedArray (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/lastIndexOf) as suggested by @DerekNonGeneric

@aduh95
Copy link
Contributor

aduh95 commented Sep 19, 2023

This would need a rebase if we still want to land it.

@aduh95 aduh95 added the stalled Issues and PRs that are stalled. label Sep 19, 2023
@github-actions
Copy link
Contributor

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

Copy link
Contributor

Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.

@github-actions github-actions bot closed this Nov 13, 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. doc Issues and PRs related to the documentations. stalled Issues and PRs that are stalled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants