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

Add a hint about deserializing helper params #205

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add a hint about deserializing helper params #205

wants to merge 1 commit into from

Conversation

flxo
Copy link

@flxo flxo commented Mar 9, 2018

This addition adds a note about the easy possiblity to work with helper
parameters in their original type representation by using Deserialize.

@flxo
Copy link
Author

flxo commented Mar 9, 2018

Sadly I had to add main to the doc text in order to use #[macro_use] for serde_derive.
I didn't want to add the example to the other helper examples because it's just a use case of the helper parameters....

src/lib.rs Outdated
//! Ok(())
//! }
//!
//! fn main() {}
Copy link
Owner

Choose a reason for hiding this comment

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

you can add # to hide this line in doc generation

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! didn't know. I will hide main and the obvious extern crate ...

@sunng87
Copy link
Owner

sunng87 commented Mar 10, 2018

hi @flxo , this looks like a good and convenient way for accessing json value, with type-safe guarantee. But it has a clone which I hope we can avoid in the doc. handlebars-rust already has too much clones that make me headache.

@flxo
Copy link
Author

flxo commented Mar 10, 2018

@sunng87 I stumbled across the clone as well. The point is that serde_json::from_value consumes a Value and ContextJson::value just borrows one that cannot be moved. If you have an idea to solve this - let me know!

I'm using this pattern in a code generator where the extra deserialisation doesn't hurt nor the clone and just had the idea to share this piece of code...

This addition adds a note about the easy possiblity to work with helper
parameters in their original type representation by using `Deserialize`.
@sunng87
Copy link
Owner

sunng87 commented Mar 10, 2018

By the way, how about adding a helper function to ContextJson that returns deserialized typed data? We can avoid the clone in this way. And I think it actually helps.

@flxo
Copy link
Author

flxo commented Mar 10, 2018

Sounds great, but I think calling clone cannot be avoided since the parameters are just borrowed in the helper and serde_json::from_value requires DeserializeOwned. Means that

 pub fn try_into(self) -> Result<T, RenderError> {

wouldn't work because it can never be called on a &ContextJson returned by param(...)

With the clone the helper function in ContextJson could look like this:

 pub fn try_into<T: ::serde::de::DeserializeOwned>(&self) -> Result<T, RenderError> {
     ::serde_json::from_value(self.value.clone())
         .map_err(|e| e.into())
}

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