-
-
Notifications
You must be signed in to change notification settings - Fork 218
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 support for warp #290
Add support for warp #290
Conversation
Hey, sorry for the slowish response to this! I've been running into trouble from having all these web framework integrations included in my template engine crate (even if optionally). This causes issues because the web framework's dependencies leak into my dep graph, causing problems if conflicting dependencies are used by the web frameworks. So, while I'm open to hosting warp integrations in this repository, I think we'd need a different approach to integrations altogether. See more discussion in #234; my attempts so far have run aground on coherence issues. Having the integration included in the web framework might be a more attractive solution, but I'm not sure if Sean et al would be open to that (do they already ship some templating integrations?). Alternatively, I need a sustainable way to do the integrations that will let me keep integrations up to date without running into dependency conflicts going forward. |
That makes sense. This feels like a place where orphan instances would be nice to have. There are currently no templating integrations in the warp repo, but I can check if they would be interested. I can't really tell whether the separate crate approach worked out or not? Is that something I should pursue for this? |
If you want to help figuring this out, that would be awesome. Currently I'm thinking that it might be better to have per-integration crates that have their own |
I haven't updated the documentation, and the mime stuff is just copy/pasted, but this seems to work. |
This is great, thanks! I'd be happy to merge this once you fix up the documentation. Also any generic mime stuff can still live in |
Also, if possible, it would be nice to have the tests in the askama_warp crate. |
Alright, I'll make the get_mime_type a |
Right now I can't figure out how to not enable that feature when running the workspace tests. |
Helpfully, I found this issue rust-lang/cargo#6195 |
Recognizing that this should never come up in the wild, maybe there's a hack that could work where we add a non-additive feature to |
Something like this? if cfg!(feature = "warp") && !cfg!(feature = "test") {
self.impl_warp_reply(&mut buf);
} |
Yes, something like this. I'd like to name the feature |
That fixes |
Have been thinking about this some more. What if we change the workspace's |
That seems to work, if you just do |
I removed the From
It looks like |
So I've redone all the other integrations as separate crates now. This has caused some churn, but hopefully rebasing your work should be straightforward -- and it provides a clear model on how to add more integrations. Thanks for working on this with me! |
(Also, please squash all your commits on this branch.) |
This ended out nicely I think. Thanks. |
The readme still needs to be updated I guess. |
Thanks! I'll make sure to update the documentation before the next release. Are you comfortable with using the wrap integration from git, or would you prefer to have a release soonish? |
No rush, I'm just playing around with it for now. |
Add support for Warp's
Reply
trait in template derivations.