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

Add support for UTF-32 #216

Merged
merged 11 commits into from
Jun 26, 2019
Merged

Add support for UTF-32 #216

merged 11 commits into from
Jun 26, 2019

Conversation

kshetline
Copy link
Contributor

This adds support for UTF-32 (also using the alias UCS-4), both little-endian and big-endian. UTF32-LE and UTF32-BE can be selected specifically, or a general UTF-32 codec with BOM-based and content-based automatic detection can be used.

Copy link
Owner

@ashtuchkin ashtuchkin left a comment

Choose a reason for hiding this comment

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

Great work, thank you Kerry! Left a few comments inline and I'll be happy to merge when its ready.

package.json Outdated Show resolved Hide resolved
encodings/utf32.js Outdated Show resolved Hide resolved
encodings/utf32.js Outdated Show resolved Hide resolved
encodings/utf32.js Outdated Show resolved Hide resolved
encodings/utf32.js Outdated Show resolved Hide resolved
encodings/utf32.js Outdated Show resolved Hide resolved
encodings/utf32.js Outdated Show resolved Hide resolved
encodings/utf32.js Show resolved Hide resolved
test/utf32-test.js Show resolved Hide resolved
@kshetline
Copy link
Contributor Author

I've made pretty much all of the changes you've suggested (I'm not sure if the all-codepoint unit tests are exactly what you had in mind), but now there are problems with compatibility with older versions of Node to try to figure out. One of them looks serious, " offset is not uint" on the use of "Buffer.readUInt32BE".

@kshetline
Copy link
Contributor Author

Problems with older versions of Node sorted out.

Copy link
Owner

@ashtuchkin ashtuchkin left a comment

Choose a reason for hiding this comment

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

Great work! see just one more comment inline.

encodings/utf32.js Outdated Show resolved Hide resolved
test/utf32-test.js Show resolved Hide resolved
@ashtuchkin ashtuchkin merged commit 841031f into ashtuchkin:master Jun 26, 2019
@ashtuchkin
Copy link
Owner

Awesome, thank you Kerry! I'll cut a new version and upload it to npm soon.

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

2 participants