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 tide integration #347

Merged
merged 3 commits into from
Jul 14, 2020
Merged

Add tide integration #347

merged 3 commits into from
Jul 14, 2020

Conversation

jbr
Copy link
Contributor

@jbr jbr commented Jul 12, 2020

Hi, and thanks for Askama! I'm excited to use this with the tide web framework, so I wrote a little integration.

I had a question about the fallibility of render() (not render_into) — I tried to figure out how to write a test of the Err variant, and I couldn't figure out what the failure conditions would be, as it's a std::fmt::Write::write_fmt into a String. Is it fairly safe to just unwrap the Result returned by render? If so, that would let us use Into<Body> which would be preferable. If not, how can I provide inputs that fail in order to test both paths?

Thanks again, and let me know if there are changes I can make to improve this PR!

@jbr jbr force-pushed the tide branch 2 times, most recently from 70df813 to 7a8b394 Compare July 12, 2020 00:05
@djc
Copy link
Owner

djc commented Jul 13, 2020

Thanks for doing the work on this, will be happy to accept this!

write_fmt() just returns fmt::Result, which by definition doesn't have any meaning attached to it. If you want to test the failure path, you could probably write a custom Write implementation that errors under some particular condition. I think unwrapping is probably bad in this case.

Copy link
Owner

@djc djc left a comment

Choose a reason for hiding this comment

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

Some little things. Let me know if you're okay fixing these up!

README.md Outdated Show resolved Hide resolved
askama_tide/LICENSE-APACHE Outdated Show resolved Hide resolved
askama_tide/Cargo.toml Show resolved Hide resolved
askama_shared/src/generator.rs Outdated Show resolved Hide resolved
book/src/integrations.md Outdated Show resolved Hide resolved
@djc
Copy link
Owner

djc commented Jul 14, 2020

(Never mind, fixed this myself since it was easy.)

@djc djc merged commit ddd62ed into djc:master Jul 14, 2020
@djc
Copy link
Owner

djc commented Jul 14, 2020

Thanks!

@djc
Copy link
Owner

djc commented Jul 14, 2020

It's on crates.io now!

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