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

added function bytes to Utf8 #4

Merged
merged 2 commits into from
Apr 11, 2024
Merged

Conversation

barrytra
Copy link
Contributor

this pull request fixes #3
new function "toUtf8" has been added in both browser.ts and index.ts files. this function efficiently converts a typedArray to Utf8 encoded string.
some new tests have also been added to verify the correctness of the code

@barrytra
Copy link
Contributor Author

@junderw pls go through it and suggest if some changes are required.

Copy link
Member

@junderw junderw left a comment

Choose a reason for hiding this comment

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

Errors on a valid UTF8 string in Japanese. See the comment.

@@ -11,6 +11,10 @@ const HEX_CODEPOINTS: (number | undefined)[] = Array(256)
const ENCODER = new TextEncoder();
const DECODER = new TextDecoder("ascii");

export function toUtf8(bytes: Uint8Array): string {
return DECODER.decode(bytes);
Copy link
Member

Choose a reason for hiding this comment

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

The TextDecoder is not returning utf8 strings.

Uint8Array.from([ 227, 129, 147, 227, 130, 147, 227, 129, 171, 227, 129, 161, 227, 129, 175 ])

This should equal "こんにちは" but it doesn't. It returns "ã\x81“ã‚“ã\x81«ã\x81¡ã\x81¯"

Can you figure out why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay.. i'll look into it

@barrytra
Copy link
Contributor Author

barrytra commented Apr 11, 2024

The reason "toUtf8" was giving a different output was TextDecoder("ascii") was converting it to string values whereas it should only be textDecoder() to convert to utf8 string. another test has been added which was creating problem. This has now been fixed and all the tests run fine. @junderw

@junderw junderw merged commit bfe4c9e into bitcoinjs:master Apr 11, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Add functionality to convert bytes to utf8 string
2 participants