-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Feature for turning off template path requirement #2722
Conversation
Cargo features cannot be negative because they must be additive. A correct implementation would be to introduce a positive Still, I'm not sure the general notion behind this PR makes sense. If you remove the discovery aspect of the dyn_templates crate, all that's left is the managing of the template instance, and a responder that knows how to use said instance. Thus, if this general functionality is desired, I would suggest a PR that instead allows someone to pass in a pre-configured instance of the engine. If the engine is preconfigured, then there's no need to search the FS. And, it doesn't require any feature shenanigans. |
I agree the feature flag is probably not the best way to do this... but I'm new to this crate.
I'm not sure that's enough. The Context struct has watchers for the path that it attaches and that's agnostic of the engine. |
I don't mean to suggest that's all the work that needs to be done, but rather that is the seed of the work. What falls from that decision needs to be handled. |
I'm starting to wonder if the better course of action here is to just rip out templating from rocket completely. Is there any special benefit beyond something like: use std::include_str;
use rocket::log::error_;
use rocket::http::{ContentType, Status};
use rocket::request::Request;
use rocket::response::{Responder, Result};
use rocket::Response;
use tera::{Context, Tera};
// This function may be wonky, but is only for illustration purposes of the code below it.
fn rocket() -> Rocket {
rocket::custom(rocket_conf)
.mount("/", routes![root_path_redirect, login])
.manage(engine()?)
}
pub struct Login(Context);
impl<'r> Responder<'r, 'r> for Login {
fn respond_to(self, request: &'r Request<'_>) -> Result<'r> {
let engine = match request.rocket().state::<Tera>() {
Some(e) => e,
None => { // should never happen, see `engine() below`
error_!("Template engine not initialized!");
return Result::Err(Status::InternalServerError)
}
};
let rendered = match engine.render("login.html", &self.0) {
Ok(html) => html,
Err(err) => {
error_!("Template render failed: {err:?}");
return Result::Err(Status::InternalServerError)
},
};
Response::build_from(rendered.respond_to(request)?)
.header(ContentType::HTML)
.ok()
}
}
pub fn engine() -> anyhow::Result<Tera> {
let mut engine = Tera::default();
engine.add_raw_template("login.html", include_str!("../../templates/login.html"))?;
Ok(engine)
}
#[get("/login")]
pub fn login() -> Login {
let mut context = Context::new();
// TODO: Fill the context
Login(context)
} I'm not sure if it's a bad idea or not to shove the engine in the State. It appears to work. |
Sure, I could generalize it to be kinda like your Maybe it would be better to just show people they can "Bring Your Own Engine" like the above. |
No, that's totally valid. Rocket's templating support brings quite a few things to the table:
Perhaps some other things? In short, it's a production-ready, safe, secure, and reliable version of what you've written above. |
I'll close this PR then, as most of my needs are met with the above minimal code.
Can you say more about this? Shouldn't the engine be doing that? |
Fixes #1792