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

Support rocket 0.5.0-rc.1 based on @flo-l's PR #495

Merged
merged 6 commits into from
Jun 15, 2021
Merged

Conversation

shritesh
Copy link
Contributor

Builds off #412

FWIW, I've pointed Rocket to git and updated the test to use the new API on top of this PR at shritesh@b9f7a58.
Thank you for this.

Originally posted by @shritesh in #412 (comment)

If someone is more invested than this PR's author, maybe just start a new PR?

Originally posted by @djc in #412 (comment)

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.

Thank you for working on this!

If you add a separate commit that bumps the askama_rocket version to an rc as well, I'd be happy to push that out to crates.io if that helps anyone.

@@ -6,11 +6,11 @@ pub use rocket::request::Request;
use rocket::response::Response;
pub use rocket::response::{Responder, Result};

pub fn respond<T: Template>(t: &T, ext: &str) -> Result<'static> {
pub fn respond<'r, 'o, 'r, T: Template>(t: &'r T, ext: &'r str) -> Result<'o> {
Copy link
Owner

Choose a reason for hiding this comment

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

Uh, is it actually useful/necessary to have two lifetimes with the same name? That seems potentially confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! It was from upstream. I have removed the subsequent lifetime.

@shritesh
Copy link
Contributor Author

shritesh commented Jun 14, 2021

I have bumped the version to 0.11.0-rc.1 and addressed the comment above.

Rocket 0.5 should be out by the end of the week. I will submit another PR pointing to the release version once that happens.

@@ -6,11 +6,11 @@ pub use rocket::request::Request;
use rocket::response::Response;
pub use rocket::response::{Responder, Result};

pub fn respond<T: Template>(t: &T, ext: &str) -> Result<'static> {
pub fn respond<'r, 'o, T: Template>(t: &'r T, ext: &'r str) -> Result<'o> {
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, I'm still confused: why do we need 'o here? Since there is no input parameter that uses 'o, couldn't we keep it as is, with the return type using 'static directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't. Thank you for pointing that out. The lifetimes are part of Rocket's Responder signature. In our case we should be able to get away with elided param lifetimes and a static out lifetime.

self.write_header(
buf,
"::askama_rocket::Responder<'askama>",
Some(vec![param]),
"::askama_rocket::Responder<'askama, 'out>",
Copy link
Owner

Choose a reason for hiding this comment

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

It seems like here, too, we can simplify to just using 'static for the output lifetime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. Fixed. Thank you.

@djc djc merged commit b318d7c into djc:main Jun 15, 2021
@danbruder
Copy link

thanks @shritesh 🙂 I was just looking to try askama and rocket 5

@jplatte
Copy link
Contributor

jplatte commented Aug 2, 2021

I just tried this, and am getting errors about the generated Responder impls only having one lifetime parameter supplied when two are required (AFAICT). Am I doing sth. wrong?

Relevant dependencies:

askama = { version = "0.10.5", features = ["with-rocket"] }
askama_rocket = "0.11.0-rc.1"
rocket = "0.5.0-rc.1"

First template derive that fails:

#[derive(Template)]
#[template(path = "index.html")]
struct IndexTemplate;

Error message:

error[E0107]: this trait takes 2 lifetime arguments but 1 lifetime argument was supplied
   --> src/routes.rs:18:10
    |
18  | #[derive(Template)]
    |          ^^^^^^^^
    |          |
    |          expected 2 lifetime arguments
    |          supplied 1 lifetime argument
    |
note: trait defined here, with 2 lifetime parameters: `'r`, `'o`
   --> /home/jplatte/.cargo/registry/src/github.com-1ecc6299db9ec823/rocket-0.5.0-rc.1/src/response/responder.rs:181:11
    |
181 | pub trait Responder<'r, 'o: 'r> {
    |           ^^^^^^^^^ --  --
    = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)
help: add missing lifetime argument
    |
18  | #[derive(Template, 'askama)]
    |                  ^^^^^^^^^

Full code: https://github.com/jplatte/turbo.fish/compare/askama

@djc
Copy link
Owner

djc commented Aug 2, 2021

Doesn't look like you're doing anything wrong. Want to file a new issue (or even better, a PR)?

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.

5 participants