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 MUtf8EncoderStream and MUtf8DecoderStream #29

Closed
wants to merge 1 commit into from
Closed

Add MUtf8EncoderStream and MUtf8DecoderStream #29

wants to merge 1 commit into from

Conversation

j4k0xb
Copy link

@j4k0xb j4k0xb commented Sep 28, 2024

Closes #24

@sciencesakura
Copy link
Owner

@j4k0xb

Thank you for your contribution, great work!

That said, I have two concerns.

First, this change introduces a dependency on the DOM for mutf-8. I aim to keep the dependencies of mutf-8 as minimal as possible. While I haven't fully thought through a solution yet, one idea could be to split the module into mutf-8 and mutf-8-streams.

The second concern is a bit more significant. There might be an issue with decoding multibyte characters in this implementation. The problem occurs when a chunk is split in the middle of a byte sequence representing a multibyte character. TextDecoder has a stream mode that handles incomplete byte sequences at the end of one chunk by combining them with the start of the next for decoding. Unfortunately, MUtf8Decoder does not yet implement stream mode.

For these reasons, I’ll need to close this pull request.

However, I encourage you to keep contributing, and I’m open to discussing possible solutions to these issues!

@j4k0xb j4k0xb deleted the streams branch October 5, 2024 13:49
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.

Add MUtf8EncoderStream and MUtf8DecoderStream
2 participants