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

Support deserialization of AA whose key type is string-based enum #1663

Merged
merged 2 commits into from Feb 2, 2017

Conversation

Projects
None yet
2 participants
@tom-tan
Contributor

tom-tan commented Jan 30, 2017

It fixes #1660.
I also added a unittest for serializing/deserializing AA whose key type is string-based enum.

I leave the case of AA with @byName because AA does not support this attribute yet (in implementation).

@tom-tan

This comment has been minimized.

Show comment
Hide comment
@tom-tan

tom-tan Feb 2, 2017

Contributor

rebased.

Contributor

tom-tan commented Feb 2, 2017

rebased.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Feb 2, 2017

Member

Thanks, looks good to merge!

I wonder if this should be extended to types that cast implicitly to string and can be converted back using T(str). For enums it also may be a good idea to provide some kind of annotation to explicitly request a certain behavior. But for now the existing change looks like a step in the right direction.

Member

s-ludwig commented Feb 2, 2017

Thanks, looks good to merge!

I wonder if this should be extended to types that cast implicitly to string and can be converted back using T(str). For enums it also may be a good idea to provide some kind of annotation to explicitly request a certain behavior. But for now the existing change looks like a step in the right direction.

@s-ludwig s-ludwig merged commit c1f28e6 into vibe-d:master Feb 2, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tom-tan tom-tan deleted the tom-tan:fix-1660 branch Feb 7, 2017

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