Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: deserialize from str for serde YAML compatibility #149

Conversation

LeoniePhiline
Copy link
Contributor

@LeoniePhiline LeoniePhiline commented Oct 31, 2024

Summary

Neither serde_yaml nor serde_yml used to be able to deserialize jiff types, due to lack of support for serialization from bytes.

Furthermore, an inconsistency between deserialization and serialization was brought forth in #138, where Serialize implementations instruct the corresponding Serializer to collect &strs, but Deserialize implementations instruct the corresponding Deserializers to provide bytes.

All serialized representations making use of no more than the ASCII character set, which makes the switch to deserializing from &str trivial.

This changeset first adds tests, proving the incompatibility.

Then, it provides a fix by hinting to Deserializers that a &str is expected to be passed to the Visitor next.

Commits

dab4b87test: verify serde YAML compatibility

Deserializing jiff types from YAML is not currently possible.

Neither serde_yaml nor its maintained fork serde_yml support deserializing from bytes.

These tests serve in both

  • proving the current incompatibility
  • verifying the fix in the following commit, which migrates Deserialize implementations to deserializer.deserialize_str.

Related to #148, #138

d69f375fix: deserialize from str for serde YAML compatibility

Deserializing jiff types from YAML was previously not possible:

Neither serde_yaml nor its maintained fork serde_yml support deserializing from bytes.

This changset migrates Deserialize implementations to deserializer.deserialize_str, thus providing serde YAML compatibility.

Fixes #138

Related to #148

@LeoniePhiline LeoniePhiline force-pushed the fix/138-serde-deserialize_bytes branch from b905b00 to ab8a82f Compare October 31, 2024 16:17
@LeoniePhiline
Copy link
Contributor Author

Please note that the failure of ci / wasm32-unknown-uknown (pull_request) is unrelated.

These seem to fail in all cases since October 17th.

@LeoniePhiline
Copy link
Contributor Author

LeoniePhiline commented Oct 31, 2024

Timezone lookup fails in cross compilation jobs.

I will switch to UTC.

Deserializing `jiff` types from YAML is not
currently possible.

Neither `serde_yaml` nor its maintained fork
`serde_yml` support deserializing from bytes.

These tests serve in both

- proving the current incompatibility
- verifying the fix in the following commit,
  which migrates `Deserialize` implementations
  to `deserializer.deserialize_str`.

Related to BurntSushi#148, BurntSushi#138
Deserializing `jiff` types from YAML was
previously not possible:

Neither `serde_yaml` nor its maintained fork
`serde_yml` support deserializing from bytes.

This changset migrates `Deserialize`
implementations to `deserializer.deserialize_str`,
thus providing serde YAML compatibility.

Fixes BurntSushi#148, BurntSushi#138
@LeoniePhiline LeoniePhiline force-pushed the fix/138-serde-deserialize_bytes branch from ab8a82f to d69f375 Compare October 31, 2024 16:24
BurntSushi pushed a commit that referenced this pull request Nov 30, 2024
Deserializing `jiff` types from YAML was previously not possible.
Namely, neither `serde_yaml` nor its maintained fork `serde_yml`
support deserializing from bytes.

This changset migrates `Deserialize` implementations to
`deserializer.deserialize_str`, thus providing serde YAML
compatibility.

There is some more discussion about this inconsistency at a higher
level in #138.

Fixes #138, Fixes #148, Closes #149
@BurntSushi BurntSushi added the rollup A PR that closes multiple open issues or other PRs. label Nov 30, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
rollup A PR that closes multiple open issues or other PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistency between serialize and deserialize in Serde
2 participants