Skip to content

Conversation

@rainyt
Copy link
Contributor

@rainyt rainyt commented Jan 14, 2026

Issues: #2018

If compiled using Haxe5, the haxe.io.Bytes file is not required.

@rainyt
Copy link
Contributor Author

rainyt commented Jan 14, 2026

Although Haxe5 has been improved to handle bytes correctly, it still handles them incorrectly in Haxe4. Therefore, when compiling with Haxe4.3 version, the Bytes.hx compatibility file of Haxe4 should be retained.

@player-03
Copy link
Contributor

I'm still not a fan of shadowing a core class. If a core class is messing with your use case, you could shadow it yourself, or request that Haxe release a patch.

@tobil4sk
Copy link
Member

I agree, shadowing the class doesn't seem like the correct fix here. It might be worth figuring out what exactly is the root cause of the broken processing, perhaps there is a nicer fix for it than shadowing a core type.

@rainyt
Copy link
Contributor Author

rainyt commented Jan 14, 2026

I'm still not a fan of shadowing a core class. If a core class is messing with your use case, you could shadow it yourself, or request that Haxe release a patch.

Okay, I don't think Haxe4 will specifically update a byte patch to 4.3.8 anymore? Nevertheless, it has already affected users before version 4.3.7, although this is a very subtle error that may not be encountered by all users.

My main consideration is whether downward compatibility is needed. If it is no longer needed, then this PR can be closed. Thank you!

@joshtynjala
Copy link
Member

I found a more targeted fix for issue #2018 that doesn't require us to shadow haxe.io.Bytes.

9b2be25

@tobil4sk
Copy link
Member

tobil4sk commented Jan 14, 2026

I found a more targeted fix for issue #2018 that doesn't require us to shadow haxe.io.Bytes.

In that case it looks like haxe 4 behaviour was correct, it makes sense for getString to throw if the byte sequence isn't valid utf-8 (unless the old decoder was broken). Maybe it would be good to have a comment explaining what is going on there with the empty catch block.

@joshtynjala
Copy link
Member

Maybe it would be good to have a comment explaining what is going on there with the empty catch block.

Okay, I'll add one!

@rainyt
Copy link
Contributor Author

rainyt commented Jan 14, 2026

Yeah, That's great, so there's no need to keep this, thank you!

@rainyt rainyt closed this Jan 14, 2026
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.

4 participants