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

Attempt to use the builtin and switch to Uint8Array #13

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

domenic
Copy link
Member

@domenic domenic commented Sep 12, 2021

I got stuck on nodejs/node#40091 making it hard to write tests for the error case, so I'm abandoning this for now, but uploading it as reference.

From some brief testing it doesn't look like we got that particular error case right in the past, either, although in a different way.

This would solve #7. Although we'd probably want to explain the unusual architecture of the package in the readme a bit more.

This also adds support for the fatal mode from the spec; previously I believe we were implicitly using replacement. Probably this is useful for work like #11.

@ThisIsMissEm
Copy link

Does this block #11 or just improve the situation overall?

@domenic
Copy link
Member Author

domenic commented Oct 12, 2022

Anything that causes the tests to pass would help with #11. It looks like this both helps and hurts; it supports more encodings (i.e., more subdirectories which that patch hasn't even started on supporting) but it causes some failures for basic stuff like Big5.

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.

2 participants