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

Adding subset result detection to node.js example #73

Merged
merged 1 commit into from
Mar 19, 2023

Conversation

yisibl
Copy link
Contributor

@yisibl yisibl commented Mar 19, 2023

No description provided.

@behdad behdad merged commit e896e27 into harfbuzz:main Mar 19, 2023
exports.hb_set_add(unicode_set, 'b'.charCodeAt(0));
exports.hb_set_add(unicode_set, 'c'.charCodeAt(0));
for (const text of SUBSET_TEXT) {
exports.hb_set_add(unicode_set, text.charCodeAt(0));
Copy link
Contributor

Choose a reason for hiding this comment

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

Though it is safe for this example (the text is all ASCII), charCodeAt gives UTF-16 code units (including surrogate pairs), you should use codePointAt instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the correction, let me fix it.

@yisibl yisibl deleted the check-subset-byte-length branch March 21, 2023 12:07
yisibl added a commit to yisibl/harfbuzzjs that referenced this pull request Mar 21, 2023
@khaledhosny says:

> `charCodeAt` gives UTF-16 code units (including surrogate pairs), you should use `codePointAt` instead.
> harfbuzz#73 (review)
yisibl added a commit to yisibl/harfbuzzjs that referenced this pull request Mar 21, 2023
@khaledhosny says:

> `charCodeAt` gives UTF-16 code units (including surrogate pairs), you should use `codePointAt` instead.
> harfbuzz#73 (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants