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: fix doc for napi_get_typedarray_info #20747

Closed
wants to merge 2 commits into from

Conversation

mhdawson
Copy link
Member

@mhdawson mhdawson commented May 15, 2018

The data pointer returned for the typedarray has
already been adjusted by the offset so it does not
point to the start of the buffer, instead id points
to the start of the first element.

I think we probably would have liked it to point to the
start of the buffer, but we can't change as that would be
a breaking change.

Update the doc to match the implementation.

Fixes: nodejs/node-addon-api#244

Checklist
  • [X ] make -j4 test (UNIX), or vcbuild test (Windows) passes
    .- [X] documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. dont-land-on-v4.x node-api Issues and PRs related to the Node-API. labels May 15, 2018
The data pointer returned for the typedarray has
already been adjusted by the offset so it does not
point to the start of the buffer, instead id points
to the start of the first element.

I think we probably would have liked it to point to the
start of the buffer, but we can't change as that would be
a breaking change.

Update the doc to match the implementation.

Fixes: nodejs/node-addon-api#244
doc/api/n-api.md Outdated
@@ -1750,10 +1750,15 @@ napi_status napi_get_typedarray_info(napi_env env,
properties to query.
- `[out] type`: Scalar datatype of the elements within the `TypedArray`.
- `[out] length`: The number of elements in the `TypedArray`.
- `[out] data`: The data buffer underlying the `TypedArray`.
- `[out] data`: The data buffer underlying the `TypedArray` adjusted by
the byte_offset value so that it points to the first element in the
Copy link
Contributor

Choose a reason for hiding this comment

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

byte_offset -> `byte_offset`?

doc/api/n-api.md Outdated
at which the first element of the arrays is located. The value for the data
parameter has already been adjusted so that data points to the first element
in the array. Therefore, the first byte of the native array would be at
data - byte_offset.
Copy link
Contributor

Choose a reason for hiding this comment

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

data - byte_offset -> `data - byte_offset`?

@mhdawson
Copy link
Member Author

Will plan to incorporate suggested changes and land when I'm back in the office on Friday,

@mhdawson
Copy link
Member Author

Pushed commit to address comments

@mhdawson
Copy link
Member Author

@mhdawson
Copy link
Member Author

CI good landing.

@mhdawson
Copy link
Member Author

Landed as 35cf008

mhdawson added a commit that referenced this pull request May 28, 2018
The data pointer returned for the typedarray has
already been adjusted by the offset so it does not
point to the start of the buffer, instead id points
to the start of the first element.

I think we probably would have liked it to point to the
start of the buffer, but we can't change as that would be
a breaking change.

Update the doc to match the implementation.

PR-URL: #20747
Fixes: nodejs/node-addon-api#244
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson mhdawson closed this May 28, 2018
MylesBorins pushed a commit that referenced this pull request May 29, 2018
The data pointer returned for the typedarray has
already been adjusted by the offset so it does not
point to the start of the buffer, instead id points
to the start of the first element.

I think we probably would have liked it to point to the
start of the buffer, but we can't change as that would be
a breaking change.

Update the doc to match the implementation.

PR-URL: #20747
Fixes: nodejs/node-addon-api#244
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 29, 2018
@mhdawson mhdawson deleted the napi-typedarray branch September 30, 2019 13:16
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. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants