Repository: mitsuhiko/minijinja (language: rust)

Issue:
Ability to add filters, functions, and tests with dynamic names 
At present, the `Environment::{add_filter, add_function, add_test}` methods make it impossible to add filter/function/tests with dynamic (`String`) names as they expect an `&'source str`. In my application, filters, functions, and tests are loaded dynamically, so I cannot possibly generate an `&str` for the names unless I go through some unsafety and keep self-references around. Alternatively, I can simply leak the `String`, but this is far from ideal.

Ideally, I could simply pass the `String` to the appropriate method and have that be it. It's not evident to me what's being saved here as a result of using an `&str` solely, so my proposal is to allow `N: Into<Cow<'source, str>>` instead, and store the name as a `Cow<'source, str>`. 


Discussion / hints:

Fix patch:
diff --git a/minijinja/src/defaults.rs b/minijinja/src/defaults.rs
index e680e9d5..f18d467f 100644
--- a/minijinja/src/defaults.rs
+++ b/minijinja/src/defaults.rs
@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::collections::BTreeMap;
 
 use crate::error::Error;
@@ -47,74 +48,86 @@ pub fn escape_formatter(out: &mut Output, state: &State, value: &Value) -> Resul
     write_escaped(out, state.auto_escape(), value)
 }
 
-pub(crate) fn get_builtin_filters() -> BTreeMap<&'static str, filters::BoxedFilter> {
+pub(crate) fn get_builtin_filters() -> BTreeMap<Cow<'static, str>, filters::BoxedFilter> {
     let mut rv = BTreeMap::new();
-    rv.insert("safe", BoxedFilter::new(filters::safe));
-    rv.insert("escape", BoxedFilter::new(filters::escape));
-    rv.insert("e", BoxedFilter::new(filters::escape));
+    rv.insert("safe".into(), BoxedFilter::new(filters::safe));
+    rv.insert("escape".into(), BoxedFilter::new(filters::escape));
+    rv.insert("e".into(), BoxedFilter::new(filters::escape));
     #[cfg(feature = "builtins")]
     {
-        rv.insert("lower", BoxedFilter::new(filters::lower));
-        rv.insert("upper", BoxedFilter::new(filters::upper));
-        rv.insert("title", BoxedFilter::new(filters::title));
-        rv.insert("replace", BoxedFilter::new(filters::replace));
-        rv.insert("length", BoxedFilter::new(filters::length));
-        rv.insert("count", BoxedFilter::new(filters::length));
-        rv.insert("dictsort", BoxedFilter::new(filters::dictsort));
-        rv.insert("items", BoxedFilter::new(filters::items));
-        rv.insert("reverse", BoxedFilter::new(filters::reverse));
-        rv.insert("trim", BoxedFilter::new(filters::trim));
-        rv.insert("join", BoxedFilter::new(filters::join));
-        rv.insert("default", BoxedFilter::new(filters::default));
-        rv.insert("round", BoxedFilter::new(filters::round));
-        rv.insert("abs", BoxedFilter::new(filters::abs));
-        rv.insert("first", BoxedFilter::new(filters::first));
-        rv.insert("last", BoxedFilter::new(filters::last));
-        rv.insert("d", BoxedFilter::new(filters::default));
-        rv.insert("list", BoxedFilter::new(filters::list));
-        rv.insert("bool", BoxedFilter::new(filters::bool));
-        rv.insert("batch", BoxedFilter::new(filters::batch));
-        rv.insert("slice", BoxedFilter::new(filters::slice));
+        rv.insert("lower".into(), BoxedFilter::new(filters::lower));
+        rv.insert("upper".into(), BoxedFilter::new(filters::upper));
+        rv.insert("title".into(), BoxedFilter::new(filters::title));
+        rv.insert("replace".into(), BoxedFilter::new(filters::replace));
+        rv.insert("length".into(), BoxedFilter::new(filters::length));
+        rv.insert("count".into(), BoxedFilter::new(filters::length));
+        rv.insert("dictsort".into(), BoxedFilter::new(filters::dictsort));
+        rv.insert("items".into(), BoxedFilter::new(filters::items));
+        rv.insert("reverse".into(), BoxedFilter::new(filters::reverse));
+        rv.insert("trim".into(), BoxedFilter::new(filters::trim));
+        rv.insert("join".into(), BoxedFilter::new(filters::join));
+        rv.insert("default".into(), BoxedFilter::new(filters::default));
+        rv.insert("round".into(), BoxedFilter::new(filters::round));
+        rv.insert("abs".into(), BoxedFilter::new(filters::abs));
+        rv.insert("first".into(), BoxedFilter::new(filters::first));
+        rv.insert("last".into(), BoxedFilter::new(filters::last));
+        rv.insert("d".into(), BoxedFilter::new(filters::default));
+        rv.insert("list".into(), BoxedFilter::new(filters::list));
+        rv.insert("bool".into(), BoxedFilter::new(filters::bool));
+        rv.insert("batch".into(), BoxedFilter::new(filters::batch));
+        rv.insert("slice".into(), BoxedFilter::new(filters::slice));
         #[cfg(feature = "json")]
         {
-            rv.insert("tojson", BoxedFilter::new(filters::tojson));
+            rv.insert("tojson".into(), BoxedFilter::new(filters::tojson));
         }
         #[cfg(feature = "urlencode")]
         {
-            rv.insert("urlencode", BoxedFilter::new(filters::urlencode));
+            rv.insert("urlencode".into(), BoxedFilter::new(filters::urlencode));
         }
     }
 
     rv
 }
 
-pub(crate) fn get_builtin_tests() -> BTreeMap<&'static str, BoxedTest> {
+pub(crate) fn get_builtin_tests() -> BTreeMap<Cow<'static, str>, BoxedTest> {
     let mut rv = BTreeMap::new();
-    rv.insert("undefined", BoxedTest::new(tests::is_undefined));
-    rv.insert("defined", BoxedTest::new(tests::is_defined));
+    rv.insert("undefined".into(), BoxedTest::new(tests::is_undefined));
+    rv.insert("defined".into(), BoxedTest::new(tests::is_defined));
     #[cfg(feature = "builtins")]
     {
-        rv.insert("odd", BoxedTest::new(tests::is_odd));
-        rv.insert("even", BoxedTest::new(tests::is_even));
-        rv.insert("number", BoxedTest::new(tests::is_number));
-        rv.insert("string", BoxedTest::new(tests::is_string));
-        rv.insert("sequence", BoxedTest::new(tests::is_sequence));
-        rv.insert("mapping", BoxedTest::new(tests::is_mapping));
-        rv.insert("startingwith", BoxedTest::new(tests::is_startingwith));
-        rv.insert("endingwith", BoxedTest::new(tests::is_endingwith));
+        rv.insert("odd".into(), BoxedTest::new(tests::is_odd));
+        rv.insert("even".into(), BoxedTest::new(tests::is_even));
+        rv.insert("number".into(), BoxedTest::new(tests::is_number));
+        rv.insert("string".into(), BoxedTest::new(tests::is_string));
+        rv.insert("sequence".into(), BoxedTest::new(tests::is_sequence));
+        rv.insert("mapping".into(), BoxedTest::new(tests::is_mapping));
+        rv.insert(
+            "startingwith".into(),
+            BoxedTest::new(tests::is_startingwith),
+        );
+        rv.insert("endingwith".into(), BoxedTest::new(tests::is_endingwith));
     }
     rv
 }
 
-pub(crate) fn get_globals() -> BTreeMap<&'static str, Value> {
+pub(crate) fn get_globals() -> BTreeMap<Cow<'static, str>, Value> {
     #[allow(unused_mut)]
     let mut rv = BTreeMap::new();
     #[cfg(feature = "builtins")]
     {
         use crate::functions::{self, BoxedFunction};
-        rv.insert("range", BoxedFunction::new(functions::range).to_value());
-        rv.insert("dict", BoxedFunction::new(functions::dict).to_value());
-        rv.insert("debug", BoxedFunction::new(functions::debug).to_value());
+        rv.insert(
+            "range".into(),
+            BoxedFunction::new(functions::range).to_value(),
+        );
+        rv.insert(
+            "dict".into(),
+            BoxedFunction::new(functions::dict).to_value(),
+        );
+        rv.insert(
+            "debug".into(),
+            BoxedFunction::new(functions::debug).to_value(),
+        );
     }
 
     rv
diff --git a/minijinja/src/environment.rs b/minijinja/src/environment.rs
index 189eec16..ea8a148b 100644
--- a/minijinja/src/environment.rs
+++ b/minijinja/src/environment.rs
@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::fmt;
 use std::sync::Arc;
@@ -62,9 +63,9 @@ environment it's recommended to turn on the `source` feature and to use the
 #[derive(Clone)]
 pub struct Environment<'source> {
     templates: Source<'source>,
-    filters: BTreeMap<&'source str, filters::BoxedFilter>,
-    tests: BTreeMap<&'source str, tests::BoxedTest>,
-    pub(crate) globals: BTreeMap<&'source str, Value>,
+    filters: BTreeMap<Cow<'source, str>, filters::BoxedFilter>,
+    tests: BTreeMap<Cow<'source, str>, tests::BoxedTest>,
+    pub(crate) globals: BTreeMap<Cow<'source, str>, Value>,
     default_auto_escape: Arc<AutoEscapeFunc>,
     formatter: Arc<FormatterFunc>,
     #[cfg(feature = "debug")]
@@ -365,15 +366,17 @@ impl<'source> Environment<'source> {
     /// Filter functions are functions that can be applied to values in
     /// templates.  For details about filters have a look at
     /// [`Filter`](crate::filters::Filter).
-    pub fn add_filter<F, Rv, Args>(&mut self, name: &'source str, f: F)
+    pub fn add_filter<N, F, Rv, Args>(&mut self, name: N, f: F)
     where
+        N: Into<Cow<'source, str>>,
         // the crazy bounds here exist to enable borrowing in closures
         F: filters::Filter<Rv, Args>
             + for<'a> filters::Filter<Rv, <Args as FunctionArgs<'a>>::Output>,
         Rv: FunctionResult,
         Args: for<'a> FunctionArgs<'a>,
     {
-        self.filters.insert(name, filters::BoxedFilter::new(f));
+        self.filters
+            .insert(name.into(), filters::BoxedFilter::new(f));
     }
 
     /// Removes a filter by name.
@@ -386,14 +389,15 @@ impl<'source> Environment<'source> {
     /// Test functions are similar to filters but perform a check on a value
     /// where the return value is always true or false.  For details about tests
     /// have a look at [`Test`](crate::tests::Test).
-    pub fn add_test<F, Rv, Args>(&mut self, name: &'source str, f: F)
+    pub fn add_test<N, F, Rv, Args>(&mut self, name: N, f: F)
     where
+        N: Into<Cow<'source, str>>,
         // the crazy bounds here exist to enable borrowing in closures
         F: tests::Test<Rv, Args> + for<'a> tests::Test<Rv, <Args as FunctionArgs<'a>>::Output>,
         Rv: tests::TestResult,
         Args: for<'a> FunctionArgs<'a>,
     {
-        self.tests.insert(name, tests::BoxedTest::new(f));
+        self.tests.insert(name.into(), tests::BoxedTest::new(f));
     }
 
     /// Removes a test by name.
@@ -407,20 +411,24 @@ impl<'source> Environment<'source> {
     /// functions and other global variables share the same namespace.
     /// For more details about functions have a look at
     /// [`Function`](crate::functions::Function).
-    pub fn add_function<F, Rv, Args>(&mut self, name: &'source str, f: F)
+    pub fn add_function<N, F, Rv, Args>(&mut self, name: N, f: F)
     where
+        N: Into<Cow<'source, str>>,
         // the crazy bounds here exist to enable borrowing in closures
         F: functions::Function<Rv, Args>
             + for<'a> functions::Function<Rv, <Args as FunctionArgs<'a>>::Output>,
         Rv: FunctionResult,
         Args: for<'a> FunctionArgs<'a>,
     {
-        self.add_global(name, Value::from_function(f))
+        self.add_global(name.into(), Value::from_function(f))
     }
 
     /// Adds a global variable.
-    pub fn add_global(&mut self, name: &'source str, value: Value) {
-        self.globals.insert(name, value);
+    pub fn add_global<N>(&mut self, name: N, value: Value)
+    where
+        N: Into<Cow<'source, str>>,
+    {
+        self.globals.insert(name.into(), value);
     }
 
     /// Removes a global function or variable by name.


Test patch: