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

Input.readString should be in characters #8199

Closed
Aurel300 opened this issue Apr 22, 2019 · 3 comments
Closed

Input.readString should be in characters #8199

Aurel300 opened this issue Apr 22, 2019 · 3 comments

Comments

@Aurel300
Copy link
Member

(some:haxe.io.Input).readString(x, UTF8)

I think it would make more sense for this to read x characters rather than bytes. I don't really see a reason to read an exact number of bytes as a string (if need be this can be done with a simple read(x) and then getString), with Unicode strings this is just making it more likely to decode badly.

@RealyUniqueName
Copy link
Member

RealyUniqueName commented Apr 22, 2019

That depends on a protocol used to encode Bytes. If your protocol tells you that a string is encoded in the next N bytes, then it's more convenient to readString using the number of bytes.

@Aurel300
Copy link
Member Author

Aurel300 commented Apr 22, 2019

Again, if that's the case, you can do:

var str = input.read(nBytes).getString(0, nBytes); // or .toString() if we're keeping it

My concern is mostly that there currently no method to read N characters apart from reading bytes until you get a valid Unicode string that is of the correct length.

@ncannasse
Copy link
Member

We can't read N characters, that would be quite inefficient as we would have to read byte by byte.
Also, in binary protocols the string length is usually in bytes not in characters. Finally, that would be a very huge problem wrt backward compatibility.

Please update the unit tests accordingly and close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants