-
Notifications
You must be signed in to change notification settings - Fork 85
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
deer
: introduce StructVisitor
#2437
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2437 +/- ##
==========================================
- Coverage 56.35% 56.11% -0.25%
==========================================
Files 340 341 +1
Lines 26417 26563 +146
Branches 384 384
==========================================
+ Hits 14888 14905 +17
- Misses 11528 11657 +129
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty comprehensive so far. Have a couple comments, but nothing stood out other than that. Thank you!
// TODO: maybe number | ||
// TODO: IdentifierVisitor (u8, u64, str, borrowed_str, string, | ||
// bytes, bytes_buf, borrowed_bytes) | ||
// TODO: test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want these todos to be more explicit? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that might be a good idea, I have those mirrored into my own personal todo list (/ project management system), but because I do not have access to Asana I cannot create those tasks there and I didn't want to pollute the GitHub issues for this sort of thing as they are mostly bug reports right now (tho I heard whispers that that might change in the future)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with this for now as a lot of this is in flux still 👍
* feat: nuke old version * feat: init deserializer * feat: error * feat: parse number * feat: deserialize more tokens * feat: deserialize more * fix: ref * chore: yoink from #2437 * feat: adapt `skip_tokens` for `justjson` * chore: imports * feat: create `ObjectAccess` * fix: compile * feat: recursion limit * feat: recursion limit grace * feat: deserialize colon * feat: deserialize array * feat: deserialize struct * feat: deserialize enum * fix: clippy * chore: remove comments * chore: temporary fix * fix: clippy * feat: implement `deserialize_identifier` * feat: pub `StackLimit`
🌟 What is the purpose of this PR?
Implements
StructVisitor
, one of the few remaining key pieces, this allows non self-describing formats (likebincode
ormsgpack
) to use arrays instead of objects to denote structs.Important: our
json
implementation does not supportvisit_array
, this is on purpose, we do not want self-describing formats to do the same as non self-describing formats. They should always prefer the object variant.The visitor has 4 methods (instead of 2):visit_none
,visit_null
,visit_object
,visit_array
.visit_none
andvisit_null
are of interest, as they allow us easily implementDefault
behaviour.> TODO: ^ the above statement should not be true, combining theOptionalVisitor
with theStructVisitor
should be enough!🔗 Related links
deer
: implementDeserialize
forcore::ops
#2422🚀 Has this modified a publishable library?
This PR:
🐾 Next steps
IdentifierVisitor
(allows for non-string identifiers for different formats)ReceivedVariant
andExpectedVariant
support for non-str typesKeyError
,MissingError
additionsContent
(GEN-34:deer
: implementContent
IR #2434)core::ops
(deer
: implementDeserialize
forcore::ops
#2422 - unblocked by this PR)justjson
for jsonIdentifierVisitor
,MissingError
,KeyError
, andContent
are in)Experiments (slated for 0.2)