mitsuhiko / minijinja

MiniJinja is a powerful but minimal dependency template engine for Rust compatible with Jinja/Jinja2
https://docs.rs/minijinja/
Apache License 2.0
1.66k stars 101 forks source link

auto impl ArgType trait for a Deserializeable value. #323

Closed cathaysia closed 1 year ago

cathaysia commented 1 year ago

Currently, all filter parameter types need to implement the ArgType trait. This is mainly to implement the from_value function. But while a variable can be convert to Value by serde::serialize, it cannot be vice versa.

Is there a possibility to deserialize the Value back into a variable? Going a step further, is it possible to implement the ArgType trait for all types that implement Deserialize ?

cathaysia commented 1 year ago

I use such codes:

impl<'a> ArgType<'a> for Time {
    type Output = Self;

    fn from_value(value: Option<&'a Value>) -> Result<Self::Output, minijinja::Error> {
        let a = serde_json::to_string(value.unwrap()).unwrap();
        Ok(serde_json::from_str(&a).unwrap())
    }
}
mitsuhiko commented 1 year ago

I think what this is calling for is probably two things:

With that one could do something like this:

fn strftime(time: ViaDeserialize<Time>) -> String {
    time.to_string()
}
cathaysia commented 1 year ago

Yes, this is also a good solution. But is there an implementation that makes the user imperceptible? For example, if Time implements the Deserialize trait, then it can be used directly:

fn strftime(time: Time) -> String {
    time.to_string()
}
mitsuhiko commented 1 year ago

I don't think this can be done without specialization.

mitsuhiko commented 1 year ago

@cathaysia I implemented this in #325. Maybe you could try this and see how it feels to you. Requires enabling the deserialization feature.

cathaysia commented 1 year ago

I used the new way instead of the previously implemented procedural macros. it's actually very good. This also fixes the issue of seq not deserializing to vec in the old implementation.

Now suppose there is a type ViaDeserialize Wouldn't it be nice to implement From for ViaDeserialize?

mitsuhiko commented 1 year ago

Wouldn't it be nice to implement From for ViaDeserialize?

It would be a breaking change at the very least and it's likely going to be quite a bit slower for quite a few setups.