-
Notifications
You must be signed in to change notification settings - Fork 10
Add support for parsing to dynamic #11
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi @lpil! Can I bump you about this PR? |
lpil
left a comment
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.
Hello! Thank you for this! Will be super useful.
I've left some notes inline. One of the main things is that new_primitive_decoder is being used a lot instead of the regular decode API. new_primitive_decoder is for decoding primitives via FFI, it's not needed to compose decoders, they already compose!
src/tom.gleam
Outdated
| } | ||
| } | ||
|
|
||
| /// Convert a parsed TOML document into a `Dynamic`. |
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.
Fill in this documentation with what it is for please 🙏
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 filled this out for both to_dynamic and parse_to_dynamic. Let me know what you think :)
src/tom.gleam
Outdated
| } | ||
|
|
||
| /// A convenience for parsing a TOML document and immediately converting it to a Dynamic | ||
| pub fn parse_dynamic(input: String) -> Result(Dynamic, ParseError) { |
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.
Same here, more documentation please.
Also let's call it parse_to_dynamic, to make it a bit clearer 🙏
Fixes #10
This PR adds the ability to either parse directly to a
dynamic(parse_dynamic) or convert an existing TOML document to a dynamic (to_dynamic). I had to do some dynamic building like we discussed on Discord, so hopefully this matches with what we discussed 😅. I've also added decoders for all custom types, so that callers do not have to use the known forms themselves if they don't want to.I've also added a JS test target to the CI, since there's FFI involved now.
Let me know if you'd like anything fixed up!