From 4de2f265070089b59f4151e8ebba5fa463002ed5 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 2 Apr 2022 12:00:47 +0300 Subject: fix: check capturing stderr of cargo, mixing it with stdout When ./check runs a command, in the runcmd_unchecked method, it collects by default stdout and stderr into on string. The two outputs get mixed. So far, this has been OK, but I'm about to make change where it won't be OK. The get_template method needs to be able to distinguish between stdout and stderr, when I change Subplot to write warnings into stderr. Sponsored-by: author --- check | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/check b/check index 533ac50..9dac5d8 100755 --- a/check +++ b/check @@ -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) @@ -179,6 +181,7 @@ class Runcmd: filename, ], stdout=PIPE, + stderr=PIPE, ).stdout.decode("UTF-8") metadata = json.loads(metadata) impls = metadata.get("impls", {}) -- cgit v1.2.1 From 9a9ad78f74c5de9cf6ef29c77506a4b21092f34a Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 2 Apr 2022 11:31:17 +0300 Subject: fix: move echo.bib out of YAML metadata in echo.md Pandoc fails to find echo bib when ./check is run: we have no way of telling Pandoc (via the pandoc::Pandoc structure), what it's working directory should be. I am about to introduce changes that will mean Subplot will fail in this case. This will all be sorted when we switch from using Pandoc to cmark_pulldown for parsing Markdown, and when we do, this commit should be reverted, but that change is too big for me at this time. Sponsored-by: author --- examples/echo/echo.md | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) 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 -- cgit v1.2.1 From 68c09d2407b329ac03fcd9f25ba293a3a6201689 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 2 Apr 2022 11:43:40 +0300 Subject: fix: daemon.yaml mark embedded file name as such Change the binding to tell Subplot that an embedded file is referred to. This silences a warning for correct scenarios, and that will soon matter when warnings are errors by default. Sponsored-by: author --- share/python/lib/daemon.yaml | 2 ++ 1 file changed, 2 insertions(+) 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: -- cgit v1.2.1 From a3d768162e70004d207a40d803f5d17eeea8593a Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 2 Apr 2022 08:58:48 +0300 Subject: refactor: collect warnings for a document in its Document Change places where warnings used to be written out so the warnings are pushed into the list in Document instead. Sponsored-by: author --- src/doc.rs | 23 +++++++++++------------ src/error.rs | 36 ++++++++++++++++++++++++++++++++++++ src/lib.rs | 2 ++ 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/doc.rs b/src/doc.rs index 2ff1484..d6ab697 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -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

( basedir: P, markdowns: Vec, @@ -344,7 +352,7 @@ 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(text.to_string())); okay = false; } } @@ -378,10 +386,7 @@ 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 +407,10 @@ 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(step.text().to_string())); okay = false; } } diff --git a/src/error.rs b/src/error.rs index bcee4ff..31fca60 100644 --- a/src/error.rs +++ b/src/error.rs @@ -334,3 +334,39 @@ impl SubplotError { /// A result type for this crate. pub type Result = std::result::Result; + +/// A warning, or non-fatal error. +/// +/// Errors prevent Subplot from producing output. Warnings don't do that. +#[derive(Debug)] +pub enum Warning { + /// Document refers to an embedded file that doesn't exist. + UnknownEmbeddedFile(String), + + /// Embedded file is not used by any scenario. + UnusedEmbeddedFile(String), + + /// Missing step implementation. + MissingStepImplementation(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, +} + +impl Warnings { + /// Append a warning to the list. + pub fn push(&mut self, w: Warning) { + self.warnings.push(w); + } + + /// Return a slice with all the warnings in the list. + pub fn warnings(&self) -> &[Warning] { + &self.warnings + } +} diff --git a/src/lib.rs b/src/lib.rs index 314ad2a..4bef83a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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; -- cgit v1.2.1 From 85212e8ddc54c6ede71912127eaf84aac403f10d Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 2 Apr 2022 09:50:39 +0300 Subject: feat! treat warnings as errors by default Add a --merciful option to subcommands for which this matters. Adjust tests to invoke subplot with that option as necessary. Sponsored-by: author --- check | 1 + src/bin/cli/mod.rs | 2 +- src/bin/subplot.rs | 81 +++++++++++++++++++++++++++++++++++------------------ src/doc.rs | 21 +++++++++++--- src/error.rs | 15 ++++++++-- subplot.md | 24 +++++++++++----- subplotlib/build.rs | 1 + 7 files changed, 103 insertions(+), 42 deletions(-) diff --git a/check b/check index 9dac5d8..f9b331c 100755 --- a/check +++ b/check @@ -178,6 +178,7 @@ class Runcmd: "metadata", "-o", "json", + "--merciful", filename, ], stdout=PIPE, diff --git a/src/bin/cli/mod.rs b/src/bin/cli/mod.rs index a16df87..2c43c39 100644 --- a/src/bin/cli/mod.rs +++ b/src/bin/cli/mod.rs @@ -35,7 +35,7 @@ impl TryFrom<&mut Document> for Metadata { type Error = subplot::SubplotError; fn try_from(doc: &mut Document) -> std::result::Result { let mut sources: Vec<_> = doc - .sources(None) + .sources(&None) .into_iter() .map(|p| filename(Some(&p))) .collect(); diff --git a/src/bin/subplot.rs b/src/bin/subplot.rs index 2ad2df1..6d098f3 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, 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, &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, 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,41 @@ impl Codegen { } } +fn load_linted_doc( + filename: &Path, + style: Style, + template: &Option, + merciful: bool, +) -> Result { + 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); + let template = template + .as_deref() + .map(Ok) + .unwrap_or_else(|| doc.template()) + .unwrap_or(""); + 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(); diff --git a/src/doc.rs b/src/doc.rs index d6ab697..db37389 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -161,6 +161,11 @@ impl<'a> Document { style: Style, template: Option<&str>, ) -> Result { + trace!( + "Document::from_file: basedir={} filename={}", + basedir.display(), + filename.display() + ); let markdowns = vec![filename.to_path_buf()]; let mut pandoc = pandoc::new(); @@ -180,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) @@ -223,7 +229,7 @@ impl<'a> Document { /// /// The sources are any files that affect the output so that if /// the source file changes, the output needs to be re-generated. - pub fn sources(&mut self, template: Option<&str>) -> Vec { + pub fn sources(&mut self, template: &Option) -> Vec { let mut names = vec![]; for x in self.meta().bindings_filenames() { @@ -352,7 +358,10 @@ impl<'a> Document { if matches!(step.types().get(name.as_str()), Some(CaptureType::File)) && !filenames.contains(&text.to_lowercase()) { - self.warnings.push(Warning::UnknownEmbeddedFile(text.to_string())); + self.warnings.push(Warning::UnknownEmbeddedFile( + scenario.title().to_string(), + text.to_string(), + )); okay = false; } } @@ -386,7 +395,8 @@ impl<'a> Document { } } for filename in filenames.iter() { - self.warnings.push(Warning::UnusedEmbeddedFile(filename.to_string())); + self.warnings + .push(Warning::UnusedEmbeddedFile(filename.to_string())); } // We always succeed. Subplot's own subplot had valid cases of @@ -410,7 +420,10 @@ impl<'a> Document { for step in scenario.steps() { if step.function().is_none() { trace!("Missing step implementation: {:?}", step.text()); - self.warnings.push(Warning::MissingStepImplementation(step.text().to_string())); + self.warnings.push(Warning::MissingStepImplementation( + scenario.title().to_string(), + step.text().to_string(), + )); okay = false; } } diff --git a/src/error.rs b/src/error.rs index 31fca60..6e7eddd 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), @@ -338,16 +342,21 @@ pub type Result = std::result::Result; /// A warning, or non-fatal error. /// /// Errors prevent Subplot from producing output. Warnings don't do that. -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum Warning { /// Document refers to an embedded file that doesn't exist. - UnknownEmbeddedFile(String), + #[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. - MissingStepImplementation(String), + #[error("Missing step implementation: \"{1}\"\n in scenario \"{0}\"")] + MissingStepImplementation(String, String), } /// A list of warnings. diff --git a/subplot.md b/subplot.md index 5098d74..18fe8d5 100644 --- a/subplot.md +++ b/subplot.md @@ -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"); } } -- cgit v1.2.1 From c9627df005b5344f437c9eaaa98bb5fe00c47313 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 2 Apr 2022 12:40:52 +0300 Subject: feat: make typesetting issues into warnings Sponsored-by: author --- src/doc.rs | 1 + src/error.rs | 11 ++++++++++- src/typeset.rs | 14 +++++++------- src/visitor/typesetting.rs | 15 ++++++++++++--- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/doc.rs b/src/doc.rs index db37389..660b83a 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -436,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. diff --git a/src/error.rs b/src/error.rs index 6e7eddd..13d6a6c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -342,7 +342,7 @@ pub type Result = std::result::Result; /// A warning, or non-fatal error. /// /// Errors prevent Subplot from producing output. Warnings don't do that. -#[derive(Debug, thiserror::Error)] +#[derive(Debug, Clone, thiserror::Error)] pub enum Warning { /// Document refers to an embedded file that doesn't exist. #[error( @@ -357,6 +357,10 @@ pub enum Warning { /// 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), } /// A list of warnings. @@ -374,6 +378,11 @@ impl Warnings { 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 diff --git a/src/typeset.rs b/src/typeset.rs index c6301e4..49b8aed 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 = 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, + warnings: &mut Warnings, ) -> (Vec, Option) { 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); } }; diff --git a/src/visitor/typesetting.rs b/src/visitor/typesetting.rs index e29a434..f2f435e 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,7 +39,7 @@ 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") { -- cgit v1.2.1 From 7db158701e781175ead38ca1e63f6115ce527cef Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 2 Apr 2022 12:45:18 +0300 Subject: feat: report markup problems during typesetting as a warning Sponsored-by: author --- src/error.rs | 12 ++++++++++++ src/typeset.rs | 16 ++++++++-------- src/visitor/typesetting.rs | 8 ++++---- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/error.rs b/src/error.rs index 13d6a6c..c93141e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -361,6 +361,18 @@ pub enum Warning { /// 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. diff --git a/src/typeset.rs b/src/typeset.rs index 49b8aed..bc86ef8 100644 --- a/src/typeset.rs +++ b/src/typeset.rs @@ -154,11 +154,11 @@ pub fn link_as_note(attr: Attr, text: Vec, 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 f2f435e..6f82c24 100644 --- a/src/visitor/typesetting.rs +++ b/src/visitor/typesetting.rs @@ -43,11 +43,11 @@ impl<'a> MutVisitor for TypesettingVisitor<'a> { } 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 @@ -61,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) } } _ => { -- cgit v1.2.1 From 7f9a8727a1f8d7f66837322262ef24c64dc8ed5c Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 2 Apr 2022 12:54:23 +0300 Subject: chore: drop unnecessary, if harmless, debugging print from test Sponsored-by: author --- src/doc.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/doc.rs b/src/doc.rs index 660b83a..bf06625 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -631,7 +631,6 @@ mod test_extract { } fn check_result(r: Result<(Option, 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() { -- cgit v1.2.1 From 67bd6febf2b8d229a217160829c201284f9e7503 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sun, 10 Apr 2022 15:35:48 +0300 Subject: use Option<&str> instead of &Option Sponsored-by: author --- src/bin/cli/mod.rs | 2 +- src/bin/subplot.rs | 27 ++++++++++++++++----------- src/doc.rs | 2 +- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/bin/cli/mod.rs b/src/bin/cli/mod.rs index 2c43c39..a16df87 100644 --- a/src/bin/cli/mod.rs +++ b/src/bin/cli/mod.rs @@ -35,7 +35,7 @@ impl TryFrom<&mut Document> for Metadata { type Error = subplot::SubplotError; fn try_from(doc: &mut Document) -> std::result::Result { let mut sources: Vec<_> = doc - .sources(&None) + .sources(None) .into_iter() .map(|p| filename(Some(&p))) .collect(); diff --git a/src/bin/subplot.rs b/src/bin/subplot.rs index 6d098f3..6a41968 100644 --- a/src/bin/subplot.rs +++ b/src/bin/subplot.rs @@ -165,7 +165,7 @@ impl Extract { } fn run(&self) -> Result<()> { - let doc = load_linted_doc(&self.filename, Style::default(), &None, self.merciful)?; + let doc = load_linted_doc(&self.filename, Style::default(), None, self.merciful)?; let files: Vec<&DataFile> = if self.embedded.is_empty() { doc.files() @@ -271,7 +271,7 @@ impl Metadata { } fn run(&self) -> Result<()> { - let mut doc = load_linted_doc(&self.filename, Style::default(), &None, self.merciful)?; + 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(), @@ -320,7 +320,7 @@ impl Docgen { trace!("PDF output chosen"); style.typeset_links_as_notes(); } - let mut doc = load_linted_doc(&self.input, style, &self.template, self.merciful)?; + 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 @@ -340,7 +340,7 @@ impl Docgen { pandoc.add_option(pandoc::PandocOption::Standalone); pandoc.add_option(pandoc::PandocOption::NumberSections); - if Self::need_output(&mut doc, &self.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()?)); @@ -366,7 +366,7 @@ impl Docgen { time.format(DATE_FORMAT).unwrap() } - fn need_output(doc: &mut subplot::Document, template: &Option, 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, @@ -453,7 +453,7 @@ impl Codegen { fn load_linted_doc( filename: &Path, style: Style, - template: &Option, + template: Option<&str>, merciful: bool, ) -> Result { let mut doc = load_document(filename, style, None)?; @@ -463,11 +463,16 @@ fn load_linted_doc( let meta = doc.meta(); trace!("Looking for template, meta={:#?}", meta); - let template = template - .as_deref() - .map(Ok) - .unwrap_or_else(|| doc.template()) - .unwrap_or(""); + + // 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)?; diff --git a/src/doc.rs b/src/doc.rs index bf06625..89b9eea 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -229,7 +229,7 @@ impl<'a> Document { /// /// The sources are any files that affect the output so that if /// the source file changes, the output needs to be re-generated. - pub fn sources(&mut self, template: &Option) -> Vec { + pub fn sources(&mut self, template: Option<&str>) -> Vec { let mut names = vec![]; for x in self.meta().bindings_filenames() { -- cgit v1.2.1