Skip to content
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

Implementing Value type #16

Merged
merged 4 commits into from
Apr 25, 2023
Merged

Implementing Value type #16

merged 4 commits into from
Apr 25, 2023

Conversation

ecton
Copy link
Member

@ecton ecton commented Apr 25, 2023

This PR adds implementations of:

  • Parsing Rsn into a Value without serde
  • Displaying a Value without serde
  • "Parsing" a Serialize into a Value
  • "Deserializing" a Value into a Deserialize

The Value type in this commit becomes a "dumb" value type, which doesn't support comments or whitespace or annotations. This is meant to be a lightweight anonymous data interchange representation. A separate type will be added to represent an Rsn document "losslessly".

  • Implement error handling

This commit adds positive-flow implementations of:

- Parsing Rsn into a Value without serde
- "Parsing" a Serialize into a Value
- "Deserializing" a Value into a Deserialize

The Value type in this commit becomes a "dumb" value type, which doesn't
support comments or whitespace or annotations. This is meant to be a
lightweight anonymous data interchange representation. A separate type
will be added to represent an Rsn document "losslessly".
This introduces fallibility to the APIs, and also removes some
allocations that were previously being done via to_string()s.

Additionally, Value now implements Display.
@ecton ecton requested a review from ModProg April 25, 2023 20:00
@ecton ecton marked this pull request as ready for review April 25, 2023 20:01
@ecton
Copy link
Member Author

ecton commented Apr 25, 2023

There's one TODO regarding option handling. I left it as I think it should -- enabling Some to be implied by having a value be present. This allows having existing files that didn't have an Option in a specific field being able to parse without updating the file to wrap the value in Some().

@ModProg
Copy link
Collaborator

ModProg commented Apr 25, 2023

Yeah Option is a bit of an interresting thing. rsn has first class enums, so it makes sense to use them. But on the other hand there are also caseses were you'd want (both for ser and des) to encode Some as the presence of a value and None as the absense. Not sure if there would be a good way to encode this through serde. The None case can be done through #[serde(skip_serializing_if = "Option::is_none")] but not sure if something similar can be done for Some. There could be a global setting when calling the serializer, but maybe we could also provide a #[serde(with = ...)] module.

@ecton ecton merged commit 3e85f0a into khonsulabs:main Apr 25, 2023
7 checks passed
@ecton ecton deleted the value branch April 25, 2023 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants