doc: fix a few n-api doc issues #13650

mhdawson · 2017-06-13T06:43:38Z

Add doc for napi_create_string_latin1().
Fix signatures where c string was specified instead of napi_value.
Fix return type of napi_callback.
Update to specify that napi_escape_handle() can only be called once
for a given scope.

Fixes: #13555
Fixes: #13556
Fixes: #13562

Checklist

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

Affected core subsystem(s)

doc, n-api

jasongin · 2017-06-13T16:13:36Z

doc/api/n-api.md

@@ -172,7 +172,7 @@ Function pointer type for user-provided native functions which are to be
 exposed to JavaScript via N-API. Callback functions should satisfy the
 following signature:
 ```C
-typedef void (*napi_callback)(napi_env, napi_callback_info);
+typedef napi_value (*napi_callback)(napi_env, napi_callback_info);


This was already fixed in #13570. You'll need to rebase.

jasongin · 2017-06-13T16:15:07Z

doc/api/n-api.md

-for the lifetime of the outer scope.
-
+This API promotes the handle to the JavaScript object so that it is valid
+for the lifetime of the outer scope. It can only be called once.  If it


once per scope ?

jasongin · 2017-06-13T16:17:43Z

doc/api/n-api.md

@@ -1325,7 +1354,7 @@ napi_status napi_create_string_utf8(napi_env env,
 ```

 - `[in] env`: The environment that the API is invoked under.
- `[in] s`: Character buffer representing a UTF8-encoded string.
+- `[in] str`: Character buffer representing a UTF8-encoded string.
 - `[in] length`: The length of the string in characters, or -1 if it is


Just noticed - this should say length in bytes, not characters.

Also fix the doc for the length parameter of napi_create_string_utf16. That length is in two-byte code units, not characters. (GitHub won't let me put a comment on that line.)

jasongin · 2017-06-13T16:18:30Z

doc/api/n-api.md

+
+- `[in] env`: The environment that the API is invoked under.
+- `[in] str`: Character buffer representing a latin1-encoded string.
+- `[in] length`: The length of the string in characters, or -1 if it is


Length in bytes. While in the case of Latin1 a character is always one byte, I think bytes is clearer in case someone is not familiar with the encoding.

jasongin

Left some comments.

RReverser · 2017-06-14T16:47:51Z

doc/api/n-api.md

                                          napi_value* result);
 ```
 - `[in] env`: The environment that the API is invoked under.
- `[in] msg`: C string representing the text to be associated with.
+- `[in] msg`: napi_value that referneces a JavaScript String to be


typo: "references"

RReverser · 2017-06-14T16:48:14Z

doc/api/n-api.md

@@ -447,7 +449,8 @@ NODE_EXTERN napi_status napi_create_range_error(napi_env env,
                                                napi_value* result);
 ```
 - `[in] env`: The environment that the API is invoked under.
- `[in] msg`: C string representing the text to be associated with.
+- `[in] msg`: napi_value that referneces a JavaScript String to be


- Add doc for napi_create_string_latin1(). - Fix signatures where c string was specified instead of napi_value. - Fix return type of napi_callback. - Update to specify that napi_escape_handle() can only be called once for a given scope. Fixes: nodejs#13555 Fixes: nodejs#13556 Fixes: nodejs#13562

mhdawson · 2017-06-15T06:04:40Z

@jasongin, @RReverser thanks for the comments. Rebased and pushed commit to address.

mhdawson · 2017-06-16T03:25:10Z

CI run: https://ci.nodejs.org/job/node-test-pull-request/8687/

mhdawson · 2017-06-16T04:50:50Z

CI run looks ok. Arm failure was unrelated, a problem doing checkout. Going to land.

mhdawson · 2017-06-16T04:52:00Z

Landed as 62e940d

- Add doc for napi_create_string_latin1(). - Fix signatures where c string was specified instead of napi_value. - Fix return type of napi_callback. - Update to specify that napi_escape_handle() can only be called once for a given scope. PR-URL: #13650 Fixes: #13555 Fixes: #13556 Fixes: #13562 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Ingvar Stepanyan <me@rreverser.com>

- Add doc for napi_create_string_latin1(). - Fix signatures where c string was specified instead of napi_value. - Fix return type of napi_callback. - Update to specify that napi_escape_handle() can only be called once for a given scope. PR-URL: nodejs#13650 Fixes: nodejs#13555 Fixes: nodejs#13556 Fixes: nodejs#13562 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Ingvar Stepanyan <me@rreverser.com>

- Add doc for napi_create_string_latin1(). - Fix signatures where c string was specified instead of napi_value. - Fix return type of napi_callback. - Update to specify that napi_escape_handle() can only be called once for a given scope. Backport-PR-URL: #19447 PR-URL: #13650 Fixes: #13555 Fixes: #13556 Fixes: #13562 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Ingvar Stepanyan <me@rreverser.com>

nodejs-github-bot added doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API. labels Jun 13, 2017

cjihrig approved these changes Jun 13, 2017

View reviewed changes

jasnell approved these changes Jun 13, 2017

View reviewed changes

jasongin reviewed Jun 13, 2017

View reviewed changes

jasongin suggested changes Jun 13, 2017

View reviewed changes

jasongin mentioned this pull request Jun 14, 2017

Explicitly restrict escapable scope to escape just one value nodejs/abi-stable-node#255

Closed

RReverser reviewed Jun 14, 2017

View reviewed changes

mhdawson added 2 commits June 15, 2017 01:55

squash: address comments

02a9e77

mhdawson force-pushed the napi-doc20 branch from bba3195 to 02a9e77 Compare June 15, 2017 06:04

RReverser self-requested a review June 15, 2017 10:52

RReverser approved these changes Jun 15, 2017

View reviewed changes

jasongin approved these changes Jun 15, 2017

View reviewed changes

mhdawson closed this Jun 16, 2017

addaleax mentioned this pull request Jun 17, 2017

v8.2.0 proposal #13744

Merged

addaleax mentioned this pull request Jun 21, 2017

v8.1.3 proposal #13861

Merged

mhdawson deleted the napi-doc20 branch June 28, 2017 19:23

MylesBorins added the dont-land-on-v6.x label Jul 17, 2017

MylesBorins mentioned this pull request Apr 16, 2018

v6.14.2 proposal #19996

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: fix a few n-api doc issues #13650

doc: fix a few n-api doc issues #13650

mhdawson commented Jun 13, 2017

jasongin Jun 13, 2017

jasongin Jun 13, 2017

jasongin Jun 13, 2017

jasongin Jun 13, 2017

jasongin Jun 13, 2017

jasongin left a comment

RReverser Jun 14, 2017 •

edited

RReverser Jun 14, 2017

mhdawson commented Jun 15, 2017

mhdawson commented Jun 16, 2017

mhdawson commented Jun 16, 2017

mhdawson commented Jun 16, 2017

doc: fix a few n-api doc issues #13650

doc: fix a few n-api doc issues #13650

Conversation

mhdawson commented Jun 13, 2017

Checklist

Affected core subsystem(s)

jasongin Jun 13, 2017

Choose a reason for hiding this comment

jasongin Jun 13, 2017

Choose a reason for hiding this comment

jasongin Jun 13, 2017

Choose a reason for hiding this comment

jasongin Jun 13, 2017

Choose a reason for hiding this comment

jasongin Jun 13, 2017

Choose a reason for hiding this comment

jasongin left a comment

Choose a reason for hiding this comment

RReverser Jun 14, 2017 • edited

Choose a reason for hiding this comment

RReverser Jun 14, 2017

Choose a reason for hiding this comment

mhdawson commented Jun 15, 2017

mhdawson commented Jun 16, 2017

mhdawson commented Jun 16, 2017

mhdawson commented Jun 16, 2017

RReverser Jun 14, 2017 •

edited