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

Implement serde's is_human_readable to return false #176

Closed
aszkid opened this issue Sep 20, 2018 · 4 comments
Closed

Implement serde's is_human_readable to return false #176

aszkid opened this issue Sep 20, 2018 · 4 comments

Comments

@aszkid
Copy link

aszkid commented Sep 20, 2018

Currently, the Serializer and Deserializer pairs in rmp-serde do not implement is_human_readable. As a consequence of this, types that support a more compact representation serialize and deserialize inefficiently: for example, IPv4 addresses should take something like 4 bytes, but currently use an annoying 17 bytes, as they serialize as regular strings (my qualm stems from serializing IpNet structs).

Has this issue been discussed before? While I do think this is a reasonable request to make, it is nevertheless a breaking change (mentioned on the serde docs—previously serialized types that support a compact representation will fail at deserialization).

I am currently using a forked repo implementing this change; merging would be extremely easy from a technical point of view (an effective 2 lines of code addded), but there is a discussion to be had.

@daboross
Copy link
Collaborator

daboross commented Dec 5, 2018

I'm in full support of this.

If msgpack-rust is against making breaking changes like this, maybe it could be done as an added option, like StructMapSerializer?

@Taywee
Copy link

Taywee commented Jun 10, 2020

I'm pretty interested in this. I'm hitting a couple snags where my msgpack format that is supposed to be compact is bigger than it should be, and I end up having to wrap in a newtype to get a compact representation where I want it instead of just being able to use a type with a human readable and non-human-readable context. Uuid is an example of this, which serializes in rmp-serde to the full 36-character hyphenated representation instead of the 16 byte array.

I could see a serializer config option working to avoid a breaking change, or a feature flag, but it would make sense at some point for this to be default behavior.

@kornelski
Copy link
Collaborator

Yes, this should definitely be behind a flag.

It's not as simple as adding fn is_human_readable to existing implementations, since the flag doesn't seem to be passed down reliably, but I was unable to find where exactly it was missing.

@joshtriplett
Copy link
Contributor

#257 (together with the serde fix in serde-rs/serde#1888 ) should fix this.

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

5 participants