Merge branch 'kinnison/fix-55' into 'master'

Add `add-newline=auto|yes|no` attribute to embedded file blocks

Closes #55

See merge request larswirzenius/subplot!46

5 files changed, 241 insertions, 15 deletions

diff --git a/src/ast.rs b/src/ast.rs
index 012e05a..bc3547d 100644
--- a/src/ast.rs
+++ b/src/ast.rs
@@ -89,6 +89,12 @@ impl<'a> Document {
     {
         let mut ast: Pandoc = serde_json::from_str(json)?;
         let meta = Metadata::new(basedir, &ast)?;
+        let mut linter = LintingVisitor::default();
+        linter.walk_pandoc(&mut ast);
+        if !linter.issues.is_empty() {
+            // Currently we can't really return more than one error so return one
+            return Err(linter.issues.remove(0));
+        }
         let files = DataFiles::new(&mut ast);
         Ok(Document::new(markdowns, ast, meta, files))
     }
@@ -970,11 +976,8 @@ pub struct DataFile {
 }
 
 impl DataFile {
-    fn new(filename: &str, contents: &str) -> DataFile {
-        DataFile {
-            filename: filename.to_string(),
-            contents: contents.to_string(),
-        }
+    fn new(filename: String, contents: String) -> DataFile {
+        DataFile { filename, contents }
     }
 
     pub fn filename(&self) -> &str {
@@ -1010,8 +1013,18 @@ impl MutVisitor for DataFiles {
             match block {
                 Block::CodeBlock(attr, contents) => {
                     if is_class(attr, "file") {
-                        self.files
-                            .push(DataFile::new(&get_filename(attr), &contents));
+                        let add_newline = match find_attr_kv(&attr, "add-newline").next() {
+                            None | Some("auto") => !contents.ends_with('\n'),
+                            Some("yes") => true,
+                            Some("no") => false,
+                            _ => unreachable!(),
+                        };
+                        let contents = if add_newline {
+                            format!("{}\n", contents)
+                        } else {
+                            contents.clone()
+                        };
+                        self.files.push(DataFile::new(get_filename(attr), contents));
                     }
                 }
                 _ => {
@@ -1070,6 +1083,41 @@ impl MutVisitor for BlockClassVisitor {
     }
 }
 
+#[derive(Default)]
+struct LintingVisitor {
+    issues: Vec<SubplotError>,
+}
+
+impl MutVisitor for LintingVisitor {
+    fn visit_vec_block(&mut self, vec_block: &mut Vec<Block>) {
+        for block in vec_block {
+            match block {
+                Block::CodeBlock(attr, _) => {
+                    if is_class(attr, "file") {
+                        let newlines: Vec<_> = find_attr_kv(&attr, "add-newline").collect();
+                        match newlines.len() {
+                            0 => {}
+                            1 => match newlines[0].to_ascii_lowercase().as_ref() {
+                                "auto" | "yes" | "no" => {}
+                                _ => self.issues.push(SubplotError::UnrecognisedAddNewline(
+                                    get_filename(&attr),
+                                    newlines[0].to_owned(),
+                                )),
+                            },
+                            _ => self.issues.push(SubplotError::RepeatedAddNewlineAttribute(
+                                get_filename(&attr),
+                            )),
+                        }
+                    }
+                }
+                _ => {
+                    self.visit_block(block);
+                }
+            }
+        }
+    }
+}
+
 /// Get the base directory given the name of the markdown file.
 ///
 /// All relative filename, such as bindings files, are resolved
@@ -1081,3 +1129,14 @@ pub fn get_basedir_from(filename: &Path) -> Result<PathBuf> {
     };
     Ok(dirname)
 }
+
+/// Utility function to find key/value pairs from an attribute
+fn find_attr_kv<'a>(attr: &'a Attr, key: &'static str) -> impl Iterator<Item = &'a str> {
+    attr.2.iter().flat_map(move |(key_, value)| {
+        if key == key_ {
+            Some(value.as_ref())
+        } else {
+            None
+        }
+    })
+}
diff --git a/src/error.rs b/src/error.rs
index 60c850e..0057f37 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -126,6 +126,20 @@ pub enum SubplotError {
     #[error("Duplicate embedded file name: {0}")]
     DuplicateEmbeddedFilename(String),
 
+    /// Embedded file has more than one `add-newline` attribute
+    ///
+    /// The `add-newline` attribute can only be specified once for any given
+    /// embedded file
+    #[error("Embedded file {0} has more than one `add-newline` attribute")]
+    RepeatedAddNewlineAttribute(String),
+
+    /// Unrecognised `add-newline` attribute value on an embedded file
+    ///
+    /// The `add-newline` attribute can only take the values `auto`, `yes`,
+    /// and `no`.
+    #[error("Embedded file {0} has unrecognised `add-newline={}` - valid values are auto/yes/no")]
+    UnrecognisedAddNewline(String, String),
+
     /// Couldn't determine base directory from input file name.
     ///
     /// Subplot needs to to determine the base directory for files
diff --git a/subplot.md b/subplot.md
index f16c6fe..ccfc7b4 100644
--- a/subplot.md
+++ b/subplot.md
@@ -1621,25 +1621,149 @@ bibliography: [foo.bib, bar.bib]
 Subplot allows data files to be embedded in the input document. This
 is handy for small test files and the like.
 
+Handling of a newline character on the last line is tricky. Pandoc
+doesn't include a newline on the last line. Sometimes one is
+needed&mdash;but sometimes it's not wanted. A newline can be added by
+having an empty line at the end, but that is subtle and easy to miss.
+Subplot helps the situation by allowing a `add-newline=` class to be added
+to the code blocks, with one of three allowed cases:
+
+* no `add-newline` class&mdash;default handling: same as `add-newline=auto`
+* `add-newline=auto`&mdash;add a newline, if one isn't there
+* `add-newline=no`&mdash;never add a newline, but keep one if it's there
+* `add-newline=yes`&mdash;always add a newline, even if one is already
+  there
+
+The scenarios below test the various cases. 
+
 ### Extract embedded file
 
-This scenario checks that an embedded file can be extracted.
+This scenario checks that an embedded file can be extracted, and used
+in a subplot.
 
 ~~~scenario
-given file onefile.md
-when I run sp-docgen onefile.md -o onefile.html
-then file onefile.html exists
+given file embedded.md
+when I run sp-docgen embedded.md -o foo.html
+then file foo.html exists
+and file foo.html matches /embedded\.txt/
 ~~~
 
-~~~~{#onefile.md .file .markdown .numberLines}
+~~~~~~~{#embedded.md .file .markdown .numberLines}
 ---
 title: One embedded file
 ...
 
-```{#filename .file}
+~~~{#embedded.txt .file}
 This is the embedded file.
-```
-~~~~
+~~~
+~~~~~~~
+
+
+### Extract embedded file, by default add missing newline
+
+This scenario checks the default handling: add a newline if one is
+missing.
+
+~~~scenario
+given file default-without-newline.txt
+then default-without-newline.txt ends in one newline
+~~~
+
+~~~{#default-without-newline.txt .file .numberLines}
+This file does not end in a newline.
+~~~
+
+
+### Extract embedded file, by default do not add a second newline
+
+This scenario checks the default handling: if content already ends in
+a newline, do not add another newline.
+
+~~~scenario
+given file default-has-newline.txt
+then default-has-newline.txt ends in one newline
+~~~
+
+~~~{#default-has-newline.txt .file .numberLines}
+This file ends in a newline.
+
+~~~
+
+### Extract embedded file, automatically add missing newline
+
+Explicitly request automatic newlines, when the file does not end in
+one.
+
+~~~scenario
+given file auto-without-newline.txt
+then auto-without-newline.txt ends in one newline
+~~~
+
+~~~{#auto-without-newline.txt .file add-newline=auto .numberLines}
+This file does not end in a newline.
+~~~
+
+
+### Extract embedded file, do not automatically add second newline
+
+Explicitly request automatic newlines, when the file already ends in
+one.
+
+~~~scenario
+given file auto-has-newline.txt
+then auto-has-newline.txt ends in one newline
+~~~
+
+~~~{#auto-has-newline.txt .file add-newline=auto .numberLines}
+This file ends in a newline.
+
+~~~
+
+
+### Extract embedded file, explicitly add missing newline
+
+Explicitly request automatic newlines, when the file doesn't end with
+one.
+
+~~~scenario
+given file add-without-newline.txt
+then add-without-newline.txt ends in one newline
+~~~
+
+~~~{#add-without-newline.txt .file add-newline=yes .numberLines}
+This file does not end in a newline.
+~~~
+
+
+### Extract embedded file, explicitly add second newline
+
+Explicitly request automatic newlines, when the file already ends with
+one.
+
+~~~scenario
+given file add-has-newline.txt
+then add-has-newline.txt ends in two newlines
+~~~
+
+~~~{#add-has-newline.txt .file add-newline=yes .numberLines}
+This file ends in a newline.
+
+~~~
+
+
+
+### Extract embedded file, do not add missing newline
+
+Explicitly ask for no newline to be added.
+
+~~~scenario
+given file no-adding-without-newline.txt
+then no-adding-without-newline.txt does not end in a newline
+~~~
+
+~~~{#no-adding-without-newline.txt .file add-newline=no .numberLines}
+This file does not end in a newline.
+~~~
 
 
 ### Fail if the same filename is used twice
diff --git a/subplot.py b/subplot.py
index bf6415f..fa3d502 100644
--- a/subplot.py
+++ b/subplot.py
@@ -107,5 +107,24 @@ def json_output_matches_file(ctx, filename=None):
     assert_dict_eq(actual, expected)
 
 
+def file_ends_in_zero_newlines(ctx, filename=None):
+    content = open(filename, "r").read()
+    print("content:", repr(content))
+    assert_ne(content[-1], "\n")
+
+
+def file_ends_in_one_newline(ctx, filename=None):
+    content = open(filename, "r").read()
+    print("content:", repr(content))
+    assert_eq(content[-1], "\n")
+    assert_ne(content[-2], "\n")
+
+
+def file_ends_in_two_newlines(ctx, filename=None):
+    content = open(filename, "r").read()
+    print("content:", repr(content))
+    assert_eq(content[-2:], "\n\n")
+
+
 def binary(basename):
     return os.path.join(srcdir, "target", "debug", basename)
diff --git a/subplot.yaml b/subplot.yaml
index da84a7d..84e52e4 100644
--- a/subplot.yaml
+++ b/subplot.yaml
@@ -55,5 +55,15 @@
   function: cleanup_was_not_run
   regex: true
 
+
 - then: JSON output matches {filename}
   function: json_output_matches_file
+
+- then: "{filename} does not end in a newline"
+  function: file_ends_in_zero_newlines
+
+- then: "{filename} ends in one newline"
+  function: file_ends_in_one_newline
+
+- then: "{filename} ends in two newlines"
+  function: file_ends_in_two_newlines