Skip to content

Conversation

cameronism
Copy link
Contributor

I think it would be useful to support serializing structs as MessagePack maps in addition to the current method of serializing them as arrays. It appears that deserialization of structs in this form already works; I didn't note any explicit logic for this case in decode.rs so I assume serde is already handling it.

I'm not crazy about the API I've exposed, this is just an initial proof of concept. If this feature is something you'd like to add, I'd be happy to discuss a better API and implementation.

It should be possible to support this feature without additional runtime branching or code duplication -- using generics. However, I think it would require a slightly different public API.

Thanks providing this great library.

@3Hren
Copy link
Owner

3Hren commented Aug 29, 2015

Hi! Thanks for contributing.

This is the part of more general case when you need special behavior while encoding particular types. See #32 for example.

I think it should be solved with policies as a template parameter to Encoder/Serializer. I'm going to investigate into this direction in 1-2 days, then I merge your PR and adapt the code.

Conflicts:
	rmp-serde/src/encode.rs
	rmp-serde/tests/serializer.rs
@cameronism
Copy link
Contributor Author

Hi! My pleasure. Thanks again for this excellent library.

I agree this is not specific to struct encoding. I updated this PR with a template / generic based solution. The changes so far are basically only to rmp-serde/src/encode.rs; if you like this implementation I could extend it to the other portions of the library. Now the PR uses static dispatch so should optimize to the same runtime behavior as master branch.

I'm new to Rust and would welcome feedback on Rust idioms and your preferences for your library.

@3Hren
Copy link
Owner

3Hren commented Aug 30, 2015

Awesome!

I've slightly adapted your changes in #39, please take a look. The changes are:

  • Returned back creating default Serializer using new function to be sure that our changes does not break any dependent code (ABI is broken of course, because of mangling, even if sizeof(Serializer) stays the same).
  • Added with function, that accepts custom struct encoding policy.
  • Custom policy has been moved into tests to be as an example how to create and integrate custom policy with Serializer.

@cameronism
Copy link
Contributor Author

Excellent. Serializer::new and Serializer::with are much nicer names.

Would you like me to close this PR and defer to #39 ?

@3Hren
Copy link
Owner

3Hren commented Aug 31, 2015

No, it will be closed automatically after I merge #39 (after minor cleanup and docs).

Thanks for participating :)

@cameronism
Copy link
Contributor Author

Happy to help 😃

@3Hren 3Hren merged commit 5df6588 into 3Hren:master Sep 1, 2015
@cameronism cameronism deleted the WIP-verbose-struct branch September 1, 2015 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants