Repository: mitsuhiko/minijinja (language: rust)

Issue:
Request: support keeping trailing newlines from templates
Right now, minijinja always strips trailing newlines from template files. The [changelog](https://github.com/mitsuhiko/minijinja/blob/9e2c0230534b905191437d02e1ecd38ccca7f435/CHANGELOG.md?plain=1#L653) notes this is done for consistency with Jinja2. Jinja2, however, has an option to enable keeping the trailing newline in the `Environment` constructor: https://jinja.palletsprojects.com/en/3.1.x/api/#jinja2.Environment minijinja is missing an option to keep them, which would be convenient and also great for consistency with Jinja2.


Discussion / hints:
Yeah I think it makes sense to add that.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1e827d65..162b157f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@ All notable changes to MiniJinja are documented here.
 
 ## 1.0.7
 
+- Added support for `keep_trailing_newlines` which allows you to disable
+  the automatic trimming of trailing newlines.  #334
 - Added `minijinja-cli` which lets you render and debug templates from
   the command line.  #331
 - Macros called with the wrong state will no longer panic.  #330
diff --git a/minijinja/src/compiler/parser.rs b/minijinja/src/compiler/parser.rs
index 55273a5c..56f1e395 100644
--- a/minijinja/src/compiler/parser.rs
+++ b/minijinja/src/compiler/parser.rs
@@ -1107,7 +1107,7 @@ impl<'a> Parser<'a> {
 /// Parses a template
 #[cfg(feature = "unstable_machinery")]
 pub fn parse<'source>(source: &'source str, filename: &str) -> Result<ast::Stmt<'source>, Error> {
-    parse_with_syntax(source, filename, Default::default())
+    parse_with_syntax(source, filename, Default::default(), false)
 }
 
 /// Parses a template with a specific syntax
@@ -1115,17 +1115,20 @@ pub fn parse_with_syntax<'source>(
     source: &'source str,
     filename: &str,
     syntax: SyntaxConfig,
+    keep_trailing_newline: bool,
 ) -> Result<ast::Stmt<'source>, Error> {
     // we want to chop off a single newline at the end.  This means that a template
     // by default does not end in a newline which is a useful property to allow
     // inline templates to work.  If someone wants a trailing newline the expectation
     // is that the user adds it themselves for achieve consistency.
     let mut source = source;
-    if source.ends_with('\n') {
-        source = &source[..source.len() - 1];
-    }
-    if source.ends_with('\r') {
-        source = &source[..source.len() - 1];
+    if !keep_trailing_newline {
+        if source.ends_with('\n') {
+            source = &source[..source.len() - 1];
+        }
+        if source.ends_with('\r') {
+            source = &source[..source.len() - 1];
+        }
     }
 
     let mut parser = Parser::new(source, false, syntax);
diff --git a/minijinja/src/environment.rs b/minijinja/src/environment.rs
index d511043b..57d51828 100644
--- a/minijinja/src/environment.rs
+++ b/minijinja/src/environment.rs
@@ -197,6 +197,19 @@ impl<'source> Environment<'source> {
         self.templates.set_loader(f);
     }
 
+    /// Preserve the trailing newline when rendering templates.
+    ///
+    /// The default is `false`, which causes a single newline, if present, to be
+    /// stripped from the end of the template.
+    pub fn set_keep_trailing_newline(&mut self, yes: bool) {
+        self.templates.keep_trailing_newline = yes;
+    }
+
+    /// Returns the value of the trailing newline preservation flag.
+    pub fn keep_trailing_newline(&self) -> bool {
+        self.templates.keep_trailing_newline
+    }
+
     /// Removes a template by name.
     pub fn remove_template(&mut self, name: &str) {
         self.templates.remove(name);
@@ -287,6 +300,7 @@ impl<'source> Environment<'source> {
                 name,
                 source,
                 self._syntax_config().clone(),
+                self.keep_trailing_newline(),
             )))),
             self.initial_auto_escape(name),
         ))
@@ -656,6 +670,7 @@ mod basic_store {
     #[derive(Clone, Default)]
     pub struct BasicStore<'source> {
         pub syntax_config: SyntaxConfig,
+        pub keep_trailing_newline: bool,
         map: BTreeMap<&'source str, Arc<CompiledTemplate<'source>>>,
     }
 
@@ -672,7 +687,8 @@ mod basic_store {
                 Arc::new(ok!(CompiledTemplate::new(
                     name,
                     source,
-                    self.syntax_config.clone()
+                    self.syntax_config.clone(),
+                    self.keep_trailing_newline
                 ))),
             );
             Ok(())
diff --git a/minijinja/src/loader.rs b/minijinja/src/loader.rs
index 18530c43..20e7a3a0 100644
--- a/minijinja/src/loader.rs
+++ b/minijinja/src/loader.rs
@@ -25,6 +25,7 @@ type LoadFunc = dyn for<'a> Fn(&'a str) -> Result<Option<String>, Error> + Send
 #[derive(Clone, Default)]
 pub(crate) struct LoaderStore<'source> {
     pub syntax_config: SyntaxConfig,
+    pub keep_trailing_newline: bool,
     loader: Option<Arc<LoadFunc>>,
     owned_templates: MemoMap<Arc<str>, Arc<LoadedTemplate>>,
     borrowed_templates: BTreeMap<&'source str, Arc<CompiledTemplate<'source>>>,
@@ -77,7 +78,8 @@ impl<'source> LoaderStore<'source> {
                     Arc::new(ok!(CompiledTemplate::new(
                         name,
                         source,
-                        self.syntax_config.clone()
+                        self.syntax_config.clone(),
+                        self.keep_trailing_newline
                     ))),
                 );
             }
@@ -137,7 +139,12 @@ impl<'source> LoaderStore<'source> {
         LoadedTemplate::try_new(
             (name, source.into_boxed_str()),
             |(name, source)| -> Result<_, Error> {
-                CompiledTemplate::new(name, source, self.syntax_config.clone())
+                CompiledTemplate::new(
+                    name,
+                    source,
+                    self.syntax_config.clone(),
+                    self.keep_trailing_newline,
+                )
             },
         )
         .map(Arc::new)
diff --git a/minijinja/src/template.rs b/minijinja/src/template.rs
index 41f8b153..a5250e0b 100644
--- a/minijinja/src/template.rs
+++ b/minijinja/src/template.rs
@@ -239,6 +239,7 @@ impl<'env, 'source> Template<'env, 'source> {
             self.compiled.instructions.source(),
             self.name(),
             self.compiled.syntax.clone(),
+            true,
         ) {
             Ok(ast) => find_undeclared(&ast, nested),
             Err(_) => HashSet::new(),
@@ -342,19 +343,29 @@ impl<'source> CompiledTemplate<'source> {
         name: &'source str,
         source: &'source str,
         syntax: SyntaxConfig,
+        keep_trailing_newline: bool,
     ) -> Result<CompiledTemplate<'source>, Error> {
-        attach_basic_debug_info(Self::_new_impl(name, source, syntax), source)
+        attach_basic_debug_info(
+            Self::_new_impl(name, source, syntax, keep_trailing_newline),
+            source,
+        )
     }
 
     fn _new_impl(
         name: &'source str,
         source: &'source str,
         syntax: SyntaxConfig,
+        keep_trailing_newline: bool,
     ) -> Result<CompiledTemplate<'source>, Error> {
         // the parser/compiler combination can create constants in which case
         // we can probably benefit from the value optimization a bit.
         let _guard = value::value_optimization();
-        let ast = ok!(parse_with_syntax(source, name, syntax.clone()));
+        let ast = ok!(parse_with_syntax(
+            source,
+            name,
+            syntax.clone(),
+            keep_trailing_newline
+        ));
         let mut gen = CodeGenerator::new(name, source);
         gen.compile_stmt(&ast);
         let buffer_size_hint = gen.buffer_size_hint();


Test patch:
diff --git a/minijinja/tests/test_environment.rs b/minijinja/tests/test_environment.rs
index 22a8545c..b635b104 100644
--- a/minijinja/tests/test_environment.rs
+++ b/minijinja/tests/test_environment.rs
@@ -113,3 +113,25 @@ fn test_path_join() {
     let t = env.get_template("x/a/foo.txt").unwrap();
     assert_eq!(t.render(()).unwrap(), "bar.txt");
 }
+
+#[test]
+fn test_keep_trailing_newlines() {
+    let mut env = Environment::new();
+    env.add_template("foo.txt", "blub\r\n").unwrap();
+    assert_eq!(env.render_str("blub\r\n", ()).unwrap(), "blub");
+
+    env.set_keep_trailing_newline(true);
+    env.add_template("foo_keep.txt", "blub\r\n").unwrap();
+    assert_eq!(
+        env.get_template("foo.txt").unwrap().render(()).unwrap(),
+        "blub"
+    );
+    assert_eq!(
+        env.get_template("foo_keep.txt")
+            .unwrap()
+            .render(())
+            .unwrap(),
+        "blub\r\n"
+    );
+    assert_eq!(env.render_str("blub\r\n", ()).unwrap(), "blub\r\n");
+}