Added closure to configure template engines #431

sunng87 · 2017-09-15T16:09:11Z

Currently there is no way to access template engine, for example, adding custom helper to handlebars.

This patch added a closure to Template::fairing which allows a closure to configure those engines.

SergioBenitez · 2017-09-16T00:01:17Z

Can you remove all of the style changes? We don't use 'rustfmt' since it doesn't quite work well enough yet.

I can review the PR once only functional changes are present.

Signed-off-by: Ning Sun <[email protected]>

sunng87 · 2017-09-16T06:00:00Z

@SergioBenitez disabled my format-on-save and commit again.

SergioBenitez · 2017-09-16T07:30:19Z

contrib/src/templates/context.rs

@@ -49,6 +49,10 @@ impl Context {
            Context { root, templates, engines }
        })
    }
+
+    pub fn engines_mut(&mut self) -> &mut Engines {


Remove this method.

SergioBenitez · 2017-09-16T07:30:30Z

contrib/src/templates/engine.rs

@@ -69,4 +69,14 @@ impl Engines {

        None
    }
+
+    #[cfg(feature = "handlebars_templates")]
+    pub fn handlebars(&mut self) -> &mut Handlebars {


Remove this method.

SergioBenitez · 2017-09-16T07:30:49Z

contrib/src/templates/engine.rs

+    }
+
+    #[cfg(feature = "tera_templates")]
+    pub fn tera(&mut self) -> &mut Tera {


Remove this method.

SergioBenitez · 2017-09-16T07:31:03Z

contrib/src/templates/mod.rs

+    ///     # ;
+    /// }
+    /// ```
+    pub fn config_and_fairing<F>(f: F) -> impl Fairing


Name this method custom.

SergioBenitez · 2017-09-16T07:31:55Z

contrib/src/templates/mod.rs

+    /// }
+    /// ```
+    pub fn config_and_fairing<F>(f: F) -> impl Fairing
+        where F: Fn(&mut Context) + Send + Sync + 'static


Pass in an &mut Engines instead. Make the fields of the Engines struct public.

Also, why does F have to be Send + Sync + 'static?

I think it Adhoc::attach requested. It won't compile if F is not marked Send + Sync + 'static

SergioBenitez · 2017-09-16T07:33:50Z

examples/handlebars_templates/src/main.rs

+        .attach(Template::config_and_fairing(|ctxt| {
+            ctxt.engines_mut()
+                .handlebars()
+                .register_helper("test",


We need a real test here. Create a helper than does something, and use it in one of the templates. Ensure that the returned value is what's expected.

SergioBenitez · 2017-09-16T07:36:31Z

examples/handlebars_templates/src/main.rs

@@ -46,6 +47,27 @@ fn rocket() -> rocket::Rocket {
        .catch(errors![not_found])
 }

+// when you want to register handlebars helpers
+#[allow(dead_code)]
+fn rocket_with_hbs_helper() -> rocket::Rocket {


Don't create a function that's not used. Instead, perhaps you can name this function something like:

fn install_helper(handlebars: &mut Handlebars) { .. }

and then call it from rocket():

.attach(Template::custom(|engines| install_helper(&mut engines.handlebars)));

SergioBenitez · 2017-09-16T07:37:10Z

contrib/src/lib.rs

@@ -74,3 +74,9 @@ mod uuid;

 #[cfg(feature = "uuid")]
 pub use uuid::{UUID, UuidParseError};
+
+#[cfg(feature = "handlebars_templates")]
+pub extern crate handlebars;


Move these extern crates to the top of the file below the existing extern crates.

Signed-off-by: Ning Sun <[email protected]>

sunng87 · 2017-09-16T08:25:52Z

examples/handlebars_templates/src/main.rs

+                                     Ok(())
+                                 }));
+
+        }))


I just changed default rocket() to use custom

Signed-off-by: Ning Sun <[email protected]>

SergioBenitez · 2017-09-16T17:26:37Z

Please see the build failure.

sunng87 · 2017-09-17T02:06:39Z

@SergioBenitez it was Template::show that has no helper config. Will a API break change on Template::show work for you? I'm going to add a closure argument to it. Of course we can a Template::custom_and_show but I think it's a little verbose for a test method. WDYT?

Signed-off-by: Ning Sun <[email protected]>

SergioBenitez · 2017-11-17T20:50:14Z

So sorry for the delay here! As we spoke about briefly in person, I think the API for Template::show needs to change a bit. The upside is that I think it'll actually be cleaner.

Here's the signature I'm imagining for Template::show():

    pub fn show<S, C>(rocket: &Rocket, name: S, context: C) -> Option<String>
        where S: Into<Cow<'static, str>>, C: Serialize
    {

The big change is that show() now takes in a Rocket instance explicitly. This means that we can use the cached versions of templates (yay!) along with their configuration (yay!) and carrying around a root path is no longer required (yay!). An instance of Rocket should be available at all points necessary. In tests, it can be obtained via client.rocket().

What do you think?

SergioBenitez · 2017-12-15T07:29:45Z

Just a heads up that #500 also seeks to implement this.

(feat) allow closure to configure template engines

bcfbea5

Signed-off-by: Ning Sun <[email protected]>

SergioBenitez requested changes Sep 16, 2017

View reviewed changes

(fix) update based on review requests

a370cc9

Signed-off-by: Ning Sun <[email protected]>

sunng87 commented Sep 16, 2017

View reviewed changes

examples/handlebars_templates/src/main.rs Outdated

Ok(())

}));

}))

Copy link

Contributor Author

sunng87 Sep 16, 2017

Choose a reason for hiding this comment

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

I just changed default rocket() to use custom

(fix) indent and helper name

1155683

Signed-off-by: Ning Sun <[email protected]>

sunng87 added 2 commits September 18, 2017 21:47

(test) added closure argument to Template::show

a07df4d

Signed-off-by: Ning Sun <[email protected]>

(fix) fix doctest

40d4c04

Signed-off-by: Ning Sun <[email protected]>

SergioBenitez mentioned this pull request Dec 14, 2017

Provide a template registration callback #500

Closed

jebrosen mentioned this pull request Dec 28, 2017

Provide template registration callback for native template engine features #521

Closed

SergioBenitez closed this in f9f1ed7 Dec 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added closure to configure template engines #431

Added closure to configure template engines #431

sunng87 commented Sep 15, 2017

SergioBenitez commented Sep 16, 2017

sunng87 commented Sep 16, 2017

SergioBenitez Sep 16, 2017

SergioBenitez Sep 16, 2017

SergioBenitez Sep 16, 2017

SergioBenitez Sep 16, 2017

SergioBenitez Sep 16, 2017

SergioBenitez Sep 16, 2017

sunng87 Sep 16, 2017

SergioBenitez Sep 16, 2017

SergioBenitez Sep 16, 2017

SergioBenitez Sep 16, 2017

sunng87 Sep 16, 2017

SergioBenitez commented Sep 16, 2017

sunng87 commented Sep 17, 2017

SergioBenitez commented Nov 17, 2017

SergioBenitez commented Dec 15, 2017

+                                                   Ok(())
+                                               }));
+                      }))

Added closure to configure template engines #431

Added closure to configure template engines #431

Conversation

sunng87 commented Sep 15, 2017

SergioBenitez commented Sep 16, 2017

sunng87 commented Sep 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergioBenitez commented Sep 16, 2017

sunng87 commented Sep 17, 2017

SergioBenitez commented Nov 17, 2017

SergioBenitez commented Dec 15, 2017