Skip to content

Conversation

grenewode
Copy link
Contributor

  • Implement serde::Serialize and serde::Deserialize for Template, TemplateBuf, ByteTemplate and ByteTemplateBuf
  • Add serde feature to crate, to control implementation of serde::Serialize and serde::Deserialize
  • make json, toml and yaml features respect serde feature

- [x] Implement `serde::Serialize` and `serde::Deserialize` for `Template`, `TemplateBuf`,
  `ByteTemplate` and `ByteTemplateBuf`
- [x] Add `serde` feature to crate, to control implementation of
  `serde::Serialize` and `serde::Deserialize`
- [x] make `json`, `toml` and `yaml` features respect `serde` feature
@de-vri-es
Copy link
Contributor

Thanks! I think this makes sense.

I think the implementation could be simplified a bit though. Rather than defining visitors you can just deserialize into a &'a str, String or byte equivalent and then parse them into a template.

Will save us a ton of code :)

}
}

impl std::cmp::Eq for Template<'_> {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Eq can also be derived.

@de-vri-es
Copy link
Contributor

I've simplified the implementations a bit by getting rid of custom visitors where possible.

I kept one for ByteTemplateBuf because Vec<u8> (de)serializes inefficiently by default (&[u8] is already fine by itself).

I don't want to use serde_test though, as it's not maintained. And it doesn't test the parsed template in detail: an implementation that sets the source but doesn't actually parse it would be accepted to (since PartialEq only compares the source).

I don't know of a great alternative now though. For deserializing there's serde::de::value::{StringDeserializer,BorrowedStrDeserializer,BorrowedBytesDeserializer}, but noticably missing is something for OwnedBytesDeserializer.

Also, there is nothing equivalent for serializing.

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.

2 participants