diff options
author | Daniel Silverstone <dsilvers+gitlab@digital-scurf.org> | 2022-04-10 13:20:34 +0000 |
---|---|---|
committer | Daniel Silverstone <dsilvers+gitlab@digital-scurf.org> | 2022-04-10 13:20:34 +0000 |
commit | 60277d108b1aab5b9bc658118427aea944cbd301 (patch) | |
tree | de3f6d9f46b0706ad6796ad6ecdce9565a37b6c0 | |
parent | 94d337a1592431b64e41e9938fdf020163bb9b67 (diff) | |
parent | 67bd6febf2b8d229a217160829c201284f9e7503 (diff) | |
download | subplot-60277d108b1aab5b9bc658118427aea944cbd301.tar.gz |
Merge branch 'liw/warnings' into 'main'
Make warnings a first-class concept in Subplot
Closes #6
See merge request subplot/subplot!267
-rwxr-xr-x | check | 8 | ||||
-rw-r--r-- | examples/echo/echo.md | 12 | ||||
-rw-r--r-- | share/python/lib/daemon.yaml | 2 | ||||
-rw-r--r-- | src/bin/subplot.rs | 86 | ||||
-rw-r--r-- | src/doc.rs | 38 | ||||
-rw-r--r-- | src/error.rs | 66 | ||||
-rw-r--r-- | src/lib.rs | 2 | ||||
-rw-r--r-- | src/typeset.rs | 30 | ||||
-rw-r--r-- | src/visitor/typesetting.rs | 23 | ||||
-rw-r--r-- | subplot.md | 24 | ||||
-rw-r--r-- | subplotlib/build.rs | 1 |
11 files changed, 212 insertions, 80 deletions
@@ -58,8 +58,10 @@ class Runcmd: self.msg(f"RUN: {argv} {kwargs}") if not self._verbose: - kwargs["stdout"] = PIPE - kwargs["stderr"] = STDOUT + if "stdout" not in kwargs: + kwargs["stdout"] = PIPE + if "stderr" not in kwargs: + kwargs["stderr"] = STDOUT assert "key" not in kwargs env = dict(os.environ) @@ -176,9 +178,11 @@ class Runcmd: "metadata", "-o", "json", + "--merciful", filename, ], stdout=PIPE, + stderr=PIPE, ).stdout.decode("UTF-8") metadata = json.loads(metadata) impls = metadata.get("impls", {}) diff --git a/examples/echo/echo.md b/examples/echo/echo.md index 3a991ba..495a102 100644 --- a/examples/echo/echo.md +++ b/examples/echo/echo.md @@ -4,9 +4,11 @@ author: The Subplot project bindings: [echo.yaml] impls: bash: [echo.sh] -bibliography: [echo.bib] ... +FIXME: This needs to move back into YAML: bibliography: [echo.bib] + + Introduction ============================================================================= @@ -41,12 +43,4 @@ then standard error is empty ``` -Test file - -~~~~{.file #foo.dat} -This is a test file. -Two lines. -~~~~ - - # References diff --git a/share/python/lib/daemon.yaml b/share/python/lib/daemon.yaml index 5cbc7d0..acca151 100644 --- a/share/python/lib/daemon.yaml +++ b/share/python/lib/daemon.yaml @@ -7,6 +7,8 @@ impl: python: function: _daemon_shell_script + types: + filename: file - when: I start "{path}{args:text}" as a background process as {name}, on port {port} impl: diff --git a/src/bin/subplot.rs b/src/bin/subplot.rs index 2ad2df1..6a41968 100644 --- a/src/bin/subplot.rs +++ b/src/bin/subplot.rs @@ -6,8 +6,7 @@ use anyhow::Result; use log::{debug, trace}; use structopt::StructOpt; use subplot::{ - codegen, load_document, load_document_with_pullmark, resource, DataFile, Document, MarkupOpts, - Style, + codegen, load_document, resource, DataFile, Document, MarkupOpts, Style, SubplotError, }; use time::{format_description::FormatItem, macros::format_description, OffsetDateTime}; @@ -134,6 +133,10 @@ impl Resources { /// extract all embedded files. if the output directory /// is not specified then this will extract to the current directory. struct Extract { + /// Allow warnings in document? + #[structopt(long)] + merciful: bool, + /// Directory to write extracted files to #[structopt( name = "DIR", @@ -162,7 +165,7 @@ impl Extract { } fn run(&self) -> Result<()> { - let doc = load_document(&self.filename, Style::default(), None)?; + let doc = load_linted_doc(&self.filename, Style::default(), None, self.merciful)?; let files: Vec<&DataFile> = if self.embedded.is_empty() { doc.files() @@ -249,6 +252,10 @@ impl Filter { /// document. This can then render that either as a plain text report for humans, /// or as a JSON object for further scripted processing. struct Metadata { + /// Allow warnings in document? + #[structopt(long)] + merciful: bool, + /// Form that you want the output to take #[structopt(short = "o", default_value = "plain", possible_values=&["plain", "json"])] output_format: cli::OutputFormat, @@ -264,7 +271,7 @@ impl Metadata { } fn run(&self) -> Result<()> { - let mut doc = load_document_with_pullmark(&self.filename, Style::default(), None)?; + let mut doc = load_linted_doc(&self.filename, Style::default(), None, self.merciful)?; let meta = cli::Metadata::try_from(&mut doc)?; match self.output_format { cli::OutputFormat::Plain => meta.write_out(), @@ -279,6 +286,10 @@ impl Metadata { /// /// Process a subplot document and typeset it using Pandoc. struct Docgen { + /// Allow warnings in document? + #[structopt(long)] + merciful: bool, + /// The template to use from the document. /// /// If not specified, subplot will try and find a unique template name from the document @@ -309,26 +320,7 @@ impl Docgen { trace!("PDF output chosen"); style.typeset_links_as_notes(); } - let mut doc = load_document(&self.input, style, None)?; - trace!("Got doc, now linting it"); - doc.lint()?; - trace!("Doc linted ok"); - let meta = doc.meta(); - trace!("Looking for template, meta={:#?}", meta); - let template = self - .template - .as_deref() - .map(Ok) - .unwrap_or_else(|| doc.template()) - .unwrap_or(""); - let template = template.to_string(); - trace!("Template: {:#?}", template); - if !doc.check_named_files_exist(&template)? - || !doc.check_matched_steps_have_impl(&template) - || !doc.check_embedded_files_are_used(&template)? - { - eprintln!("Continuing despite warnings"); - } + let mut doc = load_linted_doc(&self.input, style, self.template.as_deref(), self.merciful)?; let mut pandoc = pandoc::new(); // Metadata date from command line or file mtime. However, we @@ -348,7 +340,7 @@ impl Docgen { pandoc.add_option(pandoc::PandocOption::Standalone); pandoc.add_option(pandoc::PandocOption::NumberSections); - if Self::need_output(&mut doc, &template, &self.output) { + if Self::need_output(&mut doc, self.template.as_deref(), &self.output) { doc.typeset(); pandoc.set_input_format(pandoc::InputFormat::Json, vec![]); pandoc.set_input(pandoc::InputKind::Pipe(doc.ast()?)); @@ -374,13 +366,13 @@ impl Docgen { time.format(DATE_FORMAT).unwrap() } - fn need_output(doc: &mut subplot::Document, template: &str, output: &Path) -> bool { + fn need_output(doc: &mut subplot::Document, template: Option<&str>, output: &Path) -> bool { let output = match Self::mtime(output) { Err(_) => return true, Ok(ts) => ts, }; - for filename in doc.sources(Some(template)) { + for filename in doc.sources(template) { let source = match Self::mtime(&filename) { Err(_) => return true, Ok(ts) => ts, @@ -458,6 +450,46 @@ impl Codegen { } } +fn load_linted_doc( + filename: &Path, + style: Style, + template: Option<&str>, + merciful: bool, +) -> Result<Document, SubplotError> { + let mut doc = load_document(filename, style, None)?; + trace!("Got doc, now linting it"); + doc.lint()?; + trace!("Doc linted ok"); + + let meta = doc.meta(); + trace!("Looking for template, meta={:#?}", meta); + + // Get template as given to use (from command line), or from + // document, or else default to the empty string. + let template = if let Some(t) = template { + t + } else if let Ok(t) = doc.template() { + t + } else { + "" + }; + let template = template.to_string(); + trace!("Template: {:#?}", template); + doc.check_named_files_exist(&template)?; + doc.check_matched_steps_have_impl(&template); + doc.check_embedded_files_are_used(&template)?; + + for w in doc.warnings() { + eprintln!("WARNING: {}", w); + } + + if !doc.warnings().is_empty() && !merciful { + return Err(SubplotError::Warnings(doc.warnings().len())); + } + + Ok(doc) +} + fn real_main() { trace!("Starting Subplot"); let argparser = Toplevel::clap(); @@ -13,6 +13,7 @@ use crate::ScenarioStep; use crate::Style; use crate::{bindings::CaptureType, parser::parse_scenario_snippet}; use crate::{Result, SubplotError}; +use crate::{Warning, Warnings}; use std::collections::HashSet; use std::default::Default; @@ -83,6 +84,7 @@ pub struct Document { meta: Metadata, files: DataFiles, style: Style, + warnings: Warnings, } impl<'a> Document { @@ -99,9 +101,15 @@ impl<'a> Document { meta, files, style, + warnings: Warnings::default(), } } + /// Return all warnings about this document. + pub fn warnings(&self) -> &[Warning] { + self.warnings.warnings() + } + fn from_ast<P>( basedir: P, markdowns: Vec<PathBuf>, @@ -153,6 +161,11 @@ impl<'a> Document { style: Style, template: Option<&str>, ) -> Result<Document> { + trace!( + "Document::from_file: basedir={} filename={}", + basedir.display(), + filename.display() + ); let markdowns = vec![filename.to_path_buf()]; let mut pandoc = pandoc::new(); @@ -172,6 +185,7 @@ impl<'a> Document { pandoc::PandocOutput::ToBuffer(o) => o, _ => return Err(SubplotError::NotJson), }; + trace!("Pandoc was happy"); let doc = Document::from_json(basedir, markdowns, &output, style, template)?; trace!("Loaded document OK"); Ok(doc) @@ -344,7 +358,10 @@ impl<'a> Document { if matches!(step.types().get(name.as_str()), Some(CaptureType::File)) && !filenames.contains(&text.to_lowercase()) { - eprintln!("Found reference to unknown file {}", text); + self.warnings.push(Warning::UnknownEmbeddedFile( + scenario.title().to_string(), + text.to_string(), + )); okay = false; } } @@ -378,10 +395,8 @@ impl<'a> Document { } } for filename in filenames.iter() { - eprintln!( - "WARNING: embedded file is not used by any scenario: {}", - filename - ); + self.warnings + .push(Warning::UnusedEmbeddedFile(filename.to_string())); } // We always succeed. Subplot's own subplot had valid cases of @@ -402,16 +417,13 @@ impl<'a> Document { trace!("Found {} scenarios", scenarios.len()); for scenario in scenarios { trace!("Checking that steps in scenario"); - let mut said_scenario = false; for step in scenario.steps() { if step.function().is_none() { - if !said_scenario { - eprintln!("Scenario: '{}'", scenario.title()); - eprintln!(" Template: '{}'", template); - said_scenario = true; - } - eprintln!(" Step missing implementation: '{}'", step.text()); trace!("Missing step implementation: {:?}", step.text()); + self.warnings.push(Warning::MissingStepImplementation( + scenario.title().to_string(), + step.text().to_string(), + )); okay = false; } } @@ -424,6 +436,7 @@ impl<'a> Document { let mut visitor = visitor::TypesettingVisitor::new(self.style.clone(), self.meta.bindings()); visitor.walk_pandoc(&mut self.ast); + self.warnings.push_all(visitor.warnings()); } /// Return all scenarios in a document. @@ -618,7 +631,6 @@ mod test_extract { } fn check_result(r: Result<(Option<Scenario>, usize)>, title: Option<&str>, i: usize) { - eprintln!("checking result: {:?}", r); assert!(r.is_ok()); let (actual_scen, actual_i) = r.unwrap(); if title.is_none() { diff --git a/src/error.rs b/src/error.rs index bcee4ff..c93141e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -8,6 +8,10 @@ use thiserror::Error; /// Define all the kinds of errors any part of this crate can return. #[derive(Debug, Error)] pub enum SubplotError { + /// Document has non-fatal errors. + #[error("Document has {0} warnings.")] + Warnings(usize), + /// Subplot could not find a file named as a bindings file. #[error("binding file could not be found: {0}: {1}")] BindingsFileNotFound(PathBuf, std::io::Error), @@ -334,3 +338,65 @@ impl SubplotError { /// A result type for this crate. pub type Result<T> = std::result::Result<T, SubplotError>; + +/// A warning, or non-fatal error. +/// +/// Errors prevent Subplot from producing output. Warnings don't do that. +#[derive(Debug, Clone, thiserror::Error)] +pub enum Warning { + /// Document refers to an embedded file that doesn't exist. + #[error( + "Document refers to an embedded file that doesn't exist: \"{1}\"\n in scenario \"{0}\"" + )] + UnknownEmbeddedFile(String, String), + + /// Embedded file is not used by any scenario. + #[error("Embedded file is not used by any scenario: \"{0}\"")] + UnusedEmbeddedFile(String), + + /// Missing step implementation. + #[error("Missing step implementation: \"{1}\"\n in scenario \"{0}\"")] + MissingStepImplementation(String, String), + + /// Unknown binding when typesetting a scenario. + #[error("Unknown binding: {0}")] + UnknownBinding(String), + + /// Pikchr failed during typesetting. + #[error("Markup using pikchr failed: {0}")] + Pikchr(String), + + /// Dot failed during typesetting. + #[error("Markup using dot failed: {0}")] + Dot(String), + + /// Plantuml failed during typesetting. + #[error("Markup using plantuml failed: {0}")] + Plantuml(String), +} + +/// A list of warnings. +/// +/// Subplot collects warnings into this structure so that they can be +/// processed at the end. +#[derive(Debug, Default)] +pub struct Warnings { + warnings: Vec<Warning>, +} + +impl Warnings { + /// Append a warning to the list. + pub fn push(&mut self, w: Warning) { + self.warnings.push(w); + } + + /// Append all warnings from one list to another. + pub fn push_all(&mut self, mut other: Warnings) { + self.warnings.append(&mut other.warnings); + } + + /// Return a slice with all the warnings in the list. + pub fn warnings(&self) -> &[Warning] { + &self.warnings + } +} @@ -16,6 +16,8 @@ extern crate pandoc_ast_08 as pandoc_ast; mod error; pub use error::Result; pub use error::SubplotError; +pub use error::Warning; +pub use error::Warnings; pub mod resource; diff --git a/src/typeset.rs b/src/typeset.rs index c6301e4..bc86ef8 100644 --- a/src/typeset.rs +++ b/src/typeset.rs @@ -5,6 +5,7 @@ use crate::ScenarioStep; use crate::StepKind; use crate::SubplotError; use crate::{DotMarkup, GraphMarkup, PikchrMarkup, PlantumlMarkup}; +use crate::{Warning, Warnings}; use pandoc_ast::Attr; use pandoc_ast::Block; @@ -53,13 +54,13 @@ pub fn file_block(attr: &Attr, text: &str) -> Block { /// /// The snippet is given as a text string, which is parsed. It need /// not be a complete scenario, but it should consist of complete steps. -pub fn scenario_snippet(bindings: &Bindings, snippet: &str) -> Block { +pub fn scenario_snippet(bindings: &Bindings, snippet: &str, warnings: &mut Warnings) -> Block { let lines = parse_scenario_snippet(snippet); let mut steps = vec![]; let mut prevkind: Option<StepKind> = None; for line in lines { - let (this, thiskind) = step(bindings, line, prevkind); + let (this, thiskind) = step(bindings, line, prevkind, warnings); steps.push(this); prevkind = thiskind; } @@ -71,6 +72,7 @@ fn step( bindings: &Bindings, text: &str, prevkind: Option<StepKind>, + warnings: &mut Warnings, ) -> (Vec<Inline>, Option<StepKind>) { let step = ScenarioStep::new_from_str(text, prevkind); if step.is_err() { @@ -84,11 +86,9 @@ fn step( let m = match bindings.find("", &step) { Ok(m) => m, Err(e) => { - eprintln!("Could not select binding: {:?}", e); - return ( - error_msg(&format!("Could not select binding for: {}", text)), - prevkind, - ); + let w = Warning::UnknownBinding(format!("{}", e)); + warnings.push(w.clone()); + return (error_msg(&format!("{}", w)), prevkind); } }; @@ -154,11 +154,11 @@ pub fn link_as_note(attr: Attr, text: Vec<Inline>, target: Target) -> Inline { /// /// If the code block which contained the pikchr contains other classes, they /// can be added to the SVG for use in later typesetting etc. -pub fn pikchr_to_block(pikchr: &str, class: Option<&str>) -> Block { +pub fn pikchr_to_block(pikchr: &str, class: Option<&str>, warnings: &mut Warnings) -> Block { match PikchrMarkup::new(pikchr, class).as_svg() { Ok(svg) => typeset_svg(svg), Err(err) => { - eprintln!("pikchr render failed: {}", err); + warnings.push(Warning::Pikchr(format!("{}", err))); error(err) } } @@ -167,11 +167,11 @@ pub fn pikchr_to_block(pikchr: &str, class: Option<&str>) -> Block { // Take a dot graph, render it as SVG, and return an AST Block // element. The Block will contain the SVG data. This allows the graph // to be rendered without referending external entities. -pub fn dot_to_block(dot: &str) -> Block { +pub fn dot_to_block(dot: &str, warnings: &mut Warnings) -> Block { match DotMarkup::new(dot).as_svg() { Ok(svg) => typeset_svg(svg), Err(err) => { - eprintln!("dot failed: {}", err); + warnings.push(Warning::Dot(format!("{}", err))); error(err) } } @@ -180,11 +180,11 @@ pub fn dot_to_block(dot: &str) -> Block { // Take a PlantUML graph, render it as SVG, and return an AST Block // element. The Block will contain the SVG data. This allows the graph // to be rendered without referending external entities. -pub fn plantuml_to_block(markup: &str) -> Block { +pub fn plantuml_to_block(markup: &str, warnings: &mut Warnings) -> Block { match PlantumlMarkup::new(markup).as_svg() { Ok(svg) => typeset_svg(svg), Err(err) => { - eprintln!("plantuml failed: {}", err); + warnings.push(Warning::Plantuml(format!("{}", err))); error(err) } } @@ -192,13 +192,13 @@ pub fn plantuml_to_block(markup: &str) -> Block { /// Typeset a project roadmap expressed as textual YAML, and render it /// as an SVG image. -pub fn roadmap_to_block(yaml: &str) -> Block { +pub fn roadmap_to_block(yaml: &str, warnings: &mut Warnings) -> Block { match roadmap::from_yaml(yaml) { Ok(ref mut roadmap) => { roadmap.set_missing_statuses(); let width = 50; match roadmap.format_as_dot(width) { - Ok(dot) => dot_to_block(&dot), + Ok(dot) => dot_to_block(&dot, warnings), Err(e) => Block::Para(vec![inlinestr(&e.to_string())]), } } diff --git a/src/visitor/typesetting.rs b/src/visitor/typesetting.rs index e29a434..6f82c24 100644 --- a/src/visitor/typesetting.rs +++ b/src/visitor/typesetting.rs @@ -1,6 +1,6 @@ use crate::panhelper; use crate::typeset; -use crate::{Bindings, Style}; +use crate::{Bindings, Style, Warnings}; use pandoc_ast::{Block, Inline, MutVisitor}; @@ -10,11 +10,20 @@ use pandoc_ast::{Block, Inline, MutVisitor}; pub struct TypesettingVisitor<'a> { style: Style, bindings: &'a Bindings, + warnings: Warnings, } impl<'a> TypesettingVisitor<'a> { pub fn new(style: Style, bindings: &'a Bindings) -> Self { - TypesettingVisitor { style, bindings } + TypesettingVisitor { + style, + bindings, + warnings: Warnings::default(), + } + } + + pub fn warnings(self) -> Warnings { + self.warnings } } @@ -30,15 +39,15 @@ impl<'a> MutVisitor for TypesettingVisitor<'a> { match block { Block::CodeBlock(attr, s) => { if is_class(attr, "scenario") { - *block = typeset::scenario_snippet(self.bindings, s) + *block = typeset::scenario_snippet(self.bindings, s, &mut self.warnings) } else if is_class(attr, "file") { *block = typeset::file_block(attr, s) } else if is_class(attr, "dot") { - *block = typeset::dot_to_block(s) + *block = typeset::dot_to_block(s, &mut self.warnings) } else if is_class(attr, "plantuml") { - *block = typeset::plantuml_to_block(s) + *block = typeset::plantuml_to_block(s, &mut self.warnings) } else if is_class(attr, "roadmap") { - *block = typeset::roadmap_to_block(s) + *block = typeset::roadmap_to_block(s, &mut self.warnings) } else if is_class(attr, "pikchr") { let other_classes: Vec<_> = attr .1 @@ -52,7 +61,7 @@ impl<'a> MutVisitor for TypesettingVisitor<'a> { Some(other_classes.join(" ")) }; let class = class.as_deref(); - *block = typeset::pikchr_to_block(s, class) + *block = typeset::pikchr_to_block(s, class, &mut self.warnings) } } _ => { @@ -509,7 +509,7 @@ could be of the form `then some output is present` or `then it exits successfull With all that in mind, a good scenario looks like -```scenario +``` given the necessary starting conditions when I do the required actions then the desired outcome is achieved @@ -628,6 +628,16 @@ This is still line number three, but would it be obvious? [roadmap]: https://crates.io/search?q=roadmap +### Use embedded file + +This scenario makes sure the sample files are used in a scenario so +that they don't cause warnings. + +~~~scenario +given file numbered-lines.txt +given file not-numbered-lines.txt +~~~ + ## Document metadata Pandoc supports, and Subplot makes use of, a [YAML metadata block][] in a @@ -809,7 +819,7 @@ given file badfilename.md and file b.yaml and file f.py and an installed subplot -when I run subplot docgen badfilename.md -o foo.pdf +when I run subplot docgen --merciful badfilename.md -o foo.pdf then file foo.pdf exists when I try to run subplot codegen --run badfilename.md -o test.py then command fails @@ -1182,7 +1192,7 @@ given file aliases.md given file b.yaml given file f.py given an installed subplot -when I run subplot docgen aliases.md -o aliases.html +when I run subplot docgen --merciful aliases.md -o aliases.html then command is successful then file aliases.html matches regex /given<[^>]*> precondition foo/ then file aliases.html matches regex /when<[^>]*> I do bar/ @@ -2572,7 +2582,7 @@ in a subplot. ~~~scenario given file embedded.md and an installed subplot -when I run subplot docgen embedded.md -o foo.html +when I run subplot docgen --merciful embedded.md -o foo.html then file foo.html exists and file foo.html matches regex /embedded\.txt/ ~~~ @@ -2753,7 +2763,7 @@ embedded files that aren't used. ~~~scenario given file unusedfile.md and an installed subplot -when I try to run subplot docgen unusedfile.md -o unusedfile.html +when I try to run subplot docgen --merciful unusedfile.md -o unusedfile.html then command is successful and file unusedfile.html exists and stderr contains "thisisnotused.txt" @@ -2894,7 +2904,7 @@ The `subplot metadata` command lists embedded files in its output. ~~~scenario given file two-embedded.md and an installed subplot -when I run subplot metadata two-embedded.md +when I run subplot metadata --merciful two-embedded.md then stdout contains "foo.txt" and stdout contains "bar.yaml" ~~~ @@ -3247,7 +3257,7 @@ It does not have a YAML metadata block. given file embedded-file.md and file expected.txt and an installed subplot -when I run subplot extract embedded-file.md foo.txt -d . +when I run subplot extract --merciful embedded-file.md foo.txt -d . then files foo.txt and expected.txt match ~~~ diff --git a/subplotlib/build.rs b/subplotlib/build.rs index d1eeefd..5b02a27 100644 --- a/subplotlib/build.rs +++ b/subplotlib/build.rs @@ -28,6 +28,7 @@ fn main() { panic!("missing include file: {}", inc.display()); } println!("cargo:rerun-if-changed={}", inc.display()); + println!("cargo:rerun-if-changed={}", entry.display()); subplot_build::codegen(Path::new(&entry)).expect("failed to generate code with Subplot"); } } |