-
Notifications
You must be signed in to change notification settings - Fork 136
Fixes #115 struct flattening #403
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
Conversation
512713c
to
8ee5179
Compare
@torkleyy thoughts? |
Codecov Report
@@ Coverage Diff @@
## master #403 +/- ##
==========================================
+ Coverage 80.31% 80.85% +0.54%
==========================================
Files 49 50 +1
Lines 6247 6393 +146
==========================================
+ Hits 5017 5169 +152
+ Misses 1230 1224 -6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Yeah I'm also unsure what drawbacks this hack might have. I think there always will be some limitations when using RON + serde (which is the only option today), simply because serde was designed for JSON. IIRC there's also untagged enums that won't work with ron. The question is is it worth having this feature? Could it prevent using someone from ron because a dependency crate uses struct flattening? I think probably not. The most common use of flatten I see is for all the ignored fields to store them in a map. I think we do support that, right? |
I once did a similar experiment for untagged enums, and they could sort of be supported by ron if
I thought so too, but unfortunately both use cases call |
I see. Well I don't think |
Re untagged enums: master...JuniperLangenstein:253-untagged-enums |
I also just thought of something else: in that prototype branch I made a while ago, struct-like things were deserialised as single-item maps into We could, in theory, make single-item maps special in the Deserialize impl for Value and convert them into a ron-specific struct-like type immediately. This would allow us to do Rust -> Value -> ron -> Rust, and the Value would be much more like a ron AST than a JSON AST. However, this would mysteriously break any single-item maps in user code that go through Value. We could, again, special case that (though this would be a bit ugly and would add even more edge cases), but as soon as such a Value would be written to ron, we'd be done. Unless we special-case the ron Deserializer impl as well. However, at this point we've got a paradox: flatten needs to interpret Hence, I think we have two options going forward with Values:
Even though a ron-AST-like Value would be nicer, I think if we want to have the best compatibility, we'd have to stick with the JSON-AST-like Value, but we could offer some convenience inspector methods to check if the Value looks e.g. like a self-describing enum |
Superseded by #455 |
Fixes #115 by allowing
ron
maps to be deserialised from structs. This is a hack. I am very unsure if this is something we wantron
to support withserde
's current model, or if we wanted to hold out for some extension toserde
that would giveDeserializer
's more control about how to handle things like flatten and untagged enums etc.CHANGELOG.md