djc / askama

Type-safe, compiled Jinja-like templates for Rust
Apache License 2.0
3.47k stars 219 forks source link

Make `String` compatible with the markdown filter. #941

Closed Wilkuu-2 closed 9 months ago

Wilkuu-2 commented 9 months ago

Hello, I recently decided to switch from Tera to Askama, and I'm enjoying it so far.

Yesterday, I stumbled upon issue #719 and decided to see if I can fix the issue with the markdown filter, so I don't have to include Comrak in my own crate as a workaround. What I found out is that the issue with using String is that the filter moves the value out of the struct. This is fine for &str considering &str is already a reference.

    /// Code generated by Template derive macro  
    fn render_into(&self, writer: &mut (impl ::std::fmt::Write + ?Sized)) -> ::askama::Result<()> {
    ...
            expr0 = &::askama::filters::markdown(::askama::Html, self.content, ::core::option::Option::None)?,
            //                                                   ^^^^^^^^^^^^ 
            //                                              Value gets moved out by filter 
    ...

Making the generated code borrow the value did not create any problems in tests.

This PR also adds a test for passing Strings to the markdown filter and simplifies the function signature for the filter to accept &str.

GuillaumeGomez commented 9 months ago

Ok, now please run cargo fmt and then squash your commits.

djc commented 9 months ago

@GuillaumeGomez if there is a single conceptual change we don't need the submitter to squash -- we can just "Squash and merge" instead.

GuillaumeGomez commented 9 months ago

Ah true. I keep forgetting about it.

djc commented 9 months ago

Thanks!

Apollo-XIV commented 9 months ago

fixed an issue for me that almost made me swap to a different templating engine before i saw this thread! thank you all :D