From 6498b2eecb21ad87069be8f501f35374745106d9 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Thu, 5 May 2022 19:28:57 +0300 Subject: refactor: drop the subplot::Result type alias Replace subplot::Result with Result. I find this now to be clearer, as I don't need to remind myself which Result is being used where. This should not be a breaking change. Sponsored-by: author --- src/bindings.rs | 20 ++++++++++------- src/diagrams.rs | 10 ++++----- src/doc.rs | 56 ++++++++++++++++++++++++++++++------------------ src/error.rs | 3 --- src/lib.rs | 1 - src/matches.rs | 10 ++++++--- src/metadata.rs | 17 ++++++++++----- src/steps.rs | 7 ++++-- src/templatespec.rs | 5 ++--- subplot-build/src/lib.rs | 4 ++-- 10 files changed, 80 insertions(+), 53 deletions(-) diff --git a/src/bindings.rs b/src/bindings.rs index 0837553..5c98297 100644 --- a/src/bindings.rs +++ b/src/bindings.rs @@ -3,7 +3,7 @@ use super::MatchedSteps; use super::PartialStep; use super::ScenarioStep; use super::StepKind; -use crate::{resource, Result, SubplotError}; +use crate::{resource, SubplotError}; use serde::{Deserialize, Serialize}; use serde_aux::prelude::*; @@ -54,7 +54,7 @@ pub enum CaptureType { impl FromStr for CaptureType { type Err = SubplotError; - fn from_str(value: &str) -> Result { + fn from_str(value: &str) -> Result { match value.to_ascii_lowercase().as_str() { "word" => Ok(Self::Word), "text" => Ok(Self::Text), @@ -168,7 +168,7 @@ impl Binding { pattern: &str, case_sensitive: bool, mut types: HashMap, - ) -> Result { + ) -> Result { let regex = RegexBuilder::new(&format!("^{}$", pattern)) .case_insensitive(!case_sensitive) .build()?; @@ -462,7 +462,7 @@ impl Bindings { } /// Add bindings from a YAML string - pub fn add_from_yaml(&mut self, yaml: &str) -> Result<()> { + pub fn add_from_yaml(&mut self, yaml: &str) -> Result<(), SubplotError> { let bindings: Vec = serde_yaml::from_str(yaml)?; for wrapper in bindings { self.add(from_hashmap(&wrapper.binding)?); @@ -477,7 +477,7 @@ impl Bindings { /// Find the binding matching a given scenario step, if there is /// exactly one. - pub fn find(&self, template: &str, step: &ScenarioStep) -> Result { + pub fn find(&self, template: &str, step: &ScenarioStep) -> Result { let mut matches: Vec = self .bindings() .iter() @@ -499,7 +499,11 @@ impl Bindings { } /// Add bindings from a file. - pub fn add_from_file

(&mut self, filename: P, template: Option<&str>) -> Result<()> + pub fn add_from_file

( + &mut self, + filename: P, + template: Option<&str>, + ) -> Result<(), SubplotError> where P: AsRef + Debug, { @@ -522,7 +526,7 @@ impl Bindings { } } -fn from_hashmap(parsed: &ParsedBinding) -> Result { +fn from_hashmap(parsed: &ParsedBinding) -> Result { let given: i32 = parsed.given.is_some().into(); let when: i32 = parsed.when.is_some().into(); let then: i32 = parsed.then.is_some().into(); @@ -796,7 +800,7 @@ fn regex_from_simple_pattern( pattern: &str, explicit_plain: bool, types: &mut HashMap, -) -> Result { +) -> Result { let pat = Regex::new(r"\{[^\s\{\}]+\}").unwrap(); let mut r = String::new(); let mut end = 0; diff --git a/src/diagrams.rs b/src/diagrams.rs index 96f5d70..264bd3f 100644 --- a/src/diagrams.rs +++ b/src/diagrams.rs @@ -1,4 +1,4 @@ -use crate::{Result, SubplotError}; +use crate::SubplotError; use std::env; use std::ffi::OsString; @@ -72,7 +72,7 @@ lazy_static! { /// for the trait. pub trait DiagramMarkup { /// Convert the markup into an SVG. - fn as_svg(&self) -> Result>; + fn as_svg(&self) -> Result, SubplotError>; } /// A code block with pikchr markup. @@ -99,7 +99,7 @@ impl PikchrMarkup { } impl DiagramMarkup for PikchrMarkup { - fn as_svg(&self) -> Result> { + fn as_svg(&self) -> Result, SubplotError> { let mut flags = pikchr::PikchrFlags::default(); flags.generate_plain_errors(); let image = pikchr::Pikchr::render(&self.markup, self.class.as_deref(), flags) @@ -130,7 +130,7 @@ impl DotMarkup { } impl DiagramMarkup for DotMarkup { - fn as_svg(&self) -> Result> { + fn as_svg(&self) -> Result, SubplotError> { let mut child = Command::new(DOT_PATH.lock().unwrap().clone()) .arg("-Tsvg") .stdin(Stdio::piped()) @@ -191,7 +191,7 @@ impl PlantumlMarkup { } impl DiagramMarkup for PlantumlMarkup { - fn as_svg(&self) -> Result> { + fn as_svg(&self) -> Result, SubplotError> { let mut cmd = Command::new(JAVA_PATH.lock().unwrap().clone()); cmd.arg("-Djava.awt.headless=true") .arg("-jar") diff --git a/src/doc.rs b/src/doc.rs index 9c5fae8..e65aff5 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -11,8 +11,8 @@ use crate::PartialStep; use crate::Scenario; use crate::ScenarioStep; use crate::Style; +use crate::SubplotError; use crate::{bindings::CaptureType, parser::parse_scenario_snippet}; -use crate::{Result, SubplotError}; use crate::{Warning, Warnings}; use std::collections::HashSet; @@ -116,7 +116,7 @@ impl<'a> Document { mut ast: Pandoc, style: Style, template: Option<&str>, - ) -> Result + ) -> Result where P: AsRef + Debug, { @@ -141,7 +141,7 @@ impl<'a> Document { json: &str, style: Style, template: Option<&str>, - ) -> Result + ) -> Result where P: AsRef + Debug, { @@ -160,7 +160,7 @@ impl<'a> Document { filename: &Path, style: Style, template: Option<&str>, - ) -> Result { + ) -> Result { trace!( "Document::from_file: basedir={} filename={}", basedir.display(), @@ -201,7 +201,7 @@ impl<'a> Document { filename: &Path, style: Style, template: Option<&str>, - ) -> Result { + ) -> Result { trace!("Parsing document with pullmark-cmark from {:?}", filename); let filename = filename.to_path_buf(); let markdown = std::fs::read_to_string(&filename)?; @@ -215,7 +215,7 @@ impl<'a> Document { /// /// This is useful in a Pandoc filter, so that the filter can give /// it back to Pandoc for typesetting. - pub fn ast(&self) -> Result { + pub fn ast(&self) -> Result { let json = serde_json::to_string(&self.ast)?; Ok(json) } @@ -275,7 +275,7 @@ impl<'a> Document { } /// Check the document for common problems. - pub fn lint(&self) -> Result<()> { + pub fn lint(&self) -> Result<(), SubplotError> { trace!("Linting document"); self.check_doc_has_title()?; self.check_filenames_are_unique()?; @@ -285,7 +285,7 @@ impl<'a> Document { } // Check that all filenames for embedded files are unique. - fn check_filenames_are_unique(&self) -> Result<()> { + fn check_filenames_are_unique(&self) -> Result<(), SubplotError> { let mut known = HashSet::new(); for filename in self.files().iter().map(|f| f.filename().to_lowercase()) { if known.contains(&filename) { @@ -297,7 +297,7 @@ impl<'a> Document { } // Check that document has a title in its metadata. - fn check_doc_has_title(&self) -> Result<()> { + fn check_doc_has_title(&self) -> Result<(), SubplotError> { if self.meta().title().is_empty() { Err(SubplotError::NoTitle) } else { @@ -306,7 +306,7 @@ impl<'a> Document { } /// Check that all the block classes in the document are known - fn check_block_classes(&self) -> Result<()> { + fn check_block_classes(&self) -> Result<(), SubplotError> { let mut visitor = visitor::BlockClassVisitor::default(); // Irritatingly we can't immutably visit the AST for some reason // This clone() is expensive and unwanted, but I'm not sure how @@ -339,7 +339,7 @@ impl<'a> Document { /// Check that all named files (in matched steps) are actually present in the /// document. - pub fn check_named_files_exist(&mut self, template: &str) -> Result { + pub fn check_named_files_exist(&mut self, template: &str) -> Result { let filenames: HashSet<_> = self .files() .iter() @@ -372,7 +372,7 @@ impl<'a> Document { } /// Check that all embedded files are used by matched steps. - pub fn check_embedded_files_are_used(&mut self, template: &str) -> Result { + pub fn check_embedded_files_are_used(&mut self, template: &str) -> Result { let mut filenames: HashSet<_> = self .files() .iter() @@ -440,7 +440,7 @@ impl<'a> Document { } /// Return all scenarios in a document. - pub fn scenarios(&mut self) -> Result> { + pub fn scenarios(&mut self) -> Result, SubplotError> { let mut visitor = visitor::StructureVisitor::new(); visitor.walk_pandoc(&mut self.ast); @@ -458,7 +458,10 @@ impl<'a> Document { } /// Return matched scenarios in a document. - pub fn matched_scenarios(&mut self, template: &str) -> Result> { + pub fn matched_scenarios( + &mut self, + template: &str, + ) -> Result, SubplotError> { let scenarios = self.scenarios()?; trace!( "Found {} scenarios, checking their bindings", @@ -472,7 +475,7 @@ impl<'a> Document { } /// Extract a template name from this document - pub fn template(&self) -> Result<&str> { + pub fn template(&self) -> Result<&str, SubplotError> { let templates: Vec<_> = self.meta().templates().collect(); if templates.len() == 1 { Ok(templates[0]) @@ -487,7 +490,11 @@ impl<'a> Document { /// Load a `Document` from a file. /// /// This version uses Pandoc to parse the Markdown. -pub fn load_document

(filename: P, style: Style, template: Option<&str>) -> Result +pub fn load_document

( + filename: P, + style: Style, + template: Option<&str>, +) -> Result where P: AsRef + Debug, { @@ -512,7 +519,7 @@ pub fn load_document_with_pullmark

( filename: P, style: Style, template: Option<&str>, -) -> Result +) -> Result where P: AsRef + Debug, { @@ -531,7 +538,11 @@ where } /// Generate code for one document. -pub fn codegen(filename: &Path, output: &Path, template: Option<&str>) -> Result { +pub fn codegen( + filename: &Path, + output: &Path, + template: Option<&str>, +) -> Result { let r = load_document_with_pullmark(filename, Style::default(), template); let mut doc = match r { Ok(doc) => doc, @@ -575,7 +586,7 @@ impl CodegenOutput { } } -fn extract_scenario(e: &[visitor::Element]) -> Result<(Option, usize)> { +fn extract_scenario(e: &[visitor::Element]) -> Result<(Option, usize), SubplotError> { if e.is_empty() { // If we get here, it's a programming error. panic!("didn't expect empty list of elements"); @@ -623,7 +634,6 @@ fn extract_scenario(e: &[visitor::Element]) -> Result<(Option, usize)> mod test_extract { use super::extract_scenario; use super::visitor::Element; - use crate::Result; use crate::Scenario; use crate::SubplotError; @@ -635,7 +645,11 @@ mod test_extract { Element::Snippet(text.to_string()) } - fn check_result(r: Result<(Option, usize)>, title: Option<&str>, i: usize) { + fn check_result( + r: Result<(Option, usize), SubplotError>, + title: Option<&str>, + i: usize, + ) { 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 c93141e..f9608a8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -336,9 +336,6 @@ 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. diff --git a/src/lib.rs b/src/lib.rs index d253765..3b4e844 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,7 +14,6 @@ extern crate pandoc_ast_07 as pandoc_ast; 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; diff --git a/src/matches.rs b/src/matches.rs index f8e527a..9130641 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -1,7 +1,7 @@ use crate::Binding; -use crate::Result; use crate::Scenario; use crate::StepKind; +use crate::SubplotError; use crate::{bindings::CaptureType, Bindings}; use std::collections::HashMap; @@ -17,8 +17,12 @@ pub struct MatchedScenario { impl MatchedScenario { /// Construct a new matched scenario - pub fn new(template: &str, scen: &Scenario, bindings: &Bindings) -> Result { - let steps: Result> = scen + pub fn new( + template: &str, + scen: &Scenario, + bindings: &Bindings, + ) -> Result { + let steps: Result, SubplotError> = scen .steps() .iter() .map(|step| bindings.find(template, step)) diff --git a/src/metadata.rs b/src/metadata.rs index 0248891..5f5e183 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -1,5 +1,4 @@ -use crate::Result; -use crate::{Bindings, TemplateSpec}; +use crate::{Bindings, SubplotError, TemplateSpec}; use std::collections::HashMap; use std::fmt::Debug; @@ -31,7 +30,11 @@ pub struct DocumentImpl { impl Metadata { /// Construct a Metadata from a Document, if possible. - pub fn new

(basedir: P, doc: &Pandoc, template: Option<&str>) -> Result + pub fn new

( + basedir: P, + doc: &Pandoc, + template: Option<&str>, + ) -> Result where P: AsRef + Debug, { @@ -152,7 +155,7 @@ fn get_bindings_filenames(map: &Mapp) -> Vec { get_paths("", map, "bindings") } -fn load_template_spec(template: &str) -> Result { +fn load_template_spec(template: &str) -> Result { let mut spec_path = PathBuf::from(template); spec_path.push("template"); spec_path.push("template.yaml"); @@ -298,7 +301,11 @@ mod test_join { } } -fn get_bindings

(filenames: &[P], bindings: &mut Bindings, template: Option<&str>) -> Result<()> +fn get_bindings

( + filenames: &[P], + bindings: &mut Bindings, + template: Option<&str>, +) -> Result<(), SubplotError> where P: AsRef + Debug, { diff --git a/src/steps.rs b/src/steps.rs index c8c1bf6..ccbc588 100644 --- a/src/steps.rs +++ b/src/steps.rs @@ -1,4 +1,4 @@ -use crate::{Result, SubplotError}; +use crate::SubplotError; use serde::{Deserialize, Serialize}; use std::fmt; @@ -47,7 +47,10 @@ impl ScenarioStep { /// /// If the step uses the "and" or "but" keyword, use the default /// step kind instead. - pub fn new_from_str(text: &str, default: Option) -> Result { + pub fn new_from_str( + text: &str, + default: Option, + ) -> Result { let mut words = text.split_whitespace(); let keyword = match words.next() { diff --git a/src/templatespec.rs b/src/templatespec.rs index 5d7150b..dbb39dc 100644 --- a/src/templatespec.rs +++ b/src/templatespec.rs @@ -1,5 +1,4 @@ use crate::resource; -use crate::Result; use crate::SubplotError; use serde::Deserialize; @@ -21,7 +20,7 @@ pub struct TemplateSpec { impl TemplateSpec { // Create a new TemplateSpec from YAML text. - fn from_yaml(yaml: &str) -> Result { + fn from_yaml(yaml: &str) -> Result { Ok(serde_yaml::from_str(yaml)?) } @@ -40,7 +39,7 @@ impl TemplateSpec { } /// Read a template.yaml file and create the corresponding TemplateSpec. - pub fn from_file(filename: &Path) -> Result { + pub fn from_file(filename: &Path) -> Result { let yaml = resource::read_as_string(filename, None)?; let spec = TemplateSpec::from_yaml(&yaml)?; let dirname = match filename.parent() { diff --git a/subplot-build/src/lib.rs b/subplot-build/src/lib.rs index 28558e8..f8ed074 100644 --- a/subplot-build/src/lib.rs +++ b/subplot-build/src/lib.rs @@ -7,7 +7,7 @@ use std::env::var_os; use std::fmt::Debug; use std::path::{Path, PathBuf}; -use subplot::{get_basedir_from, Result}; +use subplot::{get_basedir_from, SubplotError}; use tracing::{event, instrument, span, Level}; /// Generate code for one document, inside `build.rs`. @@ -31,7 +31,7 @@ use tracing::{event, instrument, span, Level}; /// # dir.close().unwrap() /// ``` #[instrument(level = "trace")] -pub fn codegen

(filename: P) -> Result<()> +pub fn codegen

(filename: P) -> Result<(), SubplotError> where P: AsRef + Debug, { -- cgit v1.2.1 From 2b7160ebdf950ef872f785336fcfe17e0c4cb347 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 6 May 2022 08:39:14 +0300 Subject: refactor! split SubplotError::IoError into more specific errors Replace SubplotError::IoError with ::Spawn, ::WriteToChild, ::WaitForChild, ::ReadFile, ::CreateFile, ::Writefile. IoError was a catchall error and as such, so generic that it didn't help the user to figure out what actually is wrong. For example, there was no indication what operation was attempted or on what file. The new error variants are specific. Sponsored-by: author --- src/codegen.rs | 13 +++++++++---- src/diagrams.rs | 29 +++++++++++++++++++++-------- src/doc.rs | 3 ++- src/error.rs | 33 +++++++++++++++++++++++---------- src/templatespec.rs | 3 ++- 5 files changed, 57 insertions(+), 24 deletions(-) diff --git a/src/codegen.rs b/src/codegen.rs index b940b4d..5c4255f 100644 --- a/src/codegen.rs +++ b/src/codegen.rs @@ -62,12 +62,15 @@ fn tera(tmplspec: &TemplateSpec, templatename: &str) -> Result Result Result<(), SubplotError> { - let mut f: File = File::create(filename)?; - f.write_all(content.as_bytes())?; + let mut f: File = File::create(filename) + .map_err(|err| SubplotError::CreateFile(filename.to_path_buf(), err))?; + f.write_all(content.as_bytes()) + .map_err(|err| SubplotError::WriteFile(filename.to_path_buf(), err))?; Ok(()) } diff --git a/src/diagrams.rs b/src/diagrams.rs index 264bd3f..2bdd0ed 100644 --- a/src/diagrams.rs +++ b/src/diagrams.rs @@ -131,15 +131,21 @@ impl DotMarkup { impl DiagramMarkup for DotMarkup { fn as_svg(&self) -> Result, SubplotError> { - let mut child = Command::new(DOT_PATH.lock().unwrap().clone()) + let path = DOT_PATH.lock().unwrap().clone(); + let mut child = Command::new(&path) .arg("-Tsvg") .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) - .spawn()?; + .spawn() + .map_err(|err| SubplotError::Spawn(path.clone(), err))?; if let Some(stdin) = child.stdin.as_mut() { - stdin.write_all(self.markup.as_bytes())?; - let output = child.wait_with_output()?; + stdin + .write_all(self.markup.as_bytes()) + .map_err(SubplotError::WriteToChild)?; + let output = child + .wait_with_output() + .map_err(SubplotError::WaitForChild)?; if output.status.success() { Ok(output.stdout) } else { @@ -192,7 +198,8 @@ impl PlantumlMarkup { impl DiagramMarkup for PlantumlMarkup { fn as_svg(&self) -> Result, SubplotError> { - let mut cmd = Command::new(JAVA_PATH.lock().unwrap().clone()); + let path = JAVA_PATH.lock().unwrap().clone(); + let mut cmd = Command::new(&path); cmd.arg("-Djava.awt.headless=true") .arg("-jar") .arg(PLANTUML_JAR_PATH.lock().unwrap().clone()) @@ -208,10 +215,16 @@ impl DiagramMarkup for PlantumlMarkup { if let Some(path) = Self::build_java_path() { cmd.env("PATH", path); } - let mut child = cmd.spawn()?; + let mut child = cmd + .spawn() + .map_err(|err| SubplotError::Spawn(path.clone(), err))?; if let Some(stdin) = child.stdin.as_mut() { - stdin.write_all(self.markup.as_bytes())?; - let output = child.wait_with_output()?; + stdin + .write_all(self.markup.as_bytes()) + .map_err(SubplotError::WriteToChild)?; + let output = child + .wait_with_output() + .map_err(SubplotError::WaitForChild)?; if output.status.success() { Ok(output.stdout) } else { diff --git a/src/doc.rs b/src/doc.rs index e65aff5..37f32d6 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -204,7 +204,8 @@ impl<'a> Document { ) -> Result { trace!("Parsing document with pullmark-cmark from {:?}", filename); let filename = filename.to_path_buf(); - let markdown = std::fs::read_to_string(&filename)?; + let markdown = std::fs::read_to_string(&filename) + .map_err(|err| SubplotError::ReadFile(filename.clone(), err))?; let ast = ast::AbstractSyntaxTree::from_str(&markdown)?; trace!("Parsed document OK"); diff --git a/src/error.rs b/src/error.rs index f9608a8..f0d76e3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -259,16 +259,29 @@ pub enum SubplotError { #[error("no scenarios were found matching the `{0}` template")] NoScenariosMatched(String), - /// I/O error - /// - /// Subplot did some I/O, and it failed. This is a generic wrapper - /// for any kind of I/O error. - #[error(transparent)] - IoError { - /// The wrapped error. - #[from] - source: std::io::Error, - }, + /// Failed to invoke a program. + #[error("Failed to invoke {0}")] + Spawn(PathBuf, #[source] std::io::Error), + + /// Failed to write to stdin of child process. + #[error("Failed to write to stdin of child process")] + WriteToChild(#[source] std::io::Error), + + /// Error when waiting for child process to finish. + #[error("Error when waiting for child process to finish")] + WaitForChild(#[source] std::io::Error), + + /// Error when reading a file. + #[error("Error when reading {0}")] + ReadFile(PathBuf, #[source] std::io::Error), + + /// Error when creating a file. + #[error("Error when creating {0}")] + CreateFile(PathBuf, #[source] std::io::Error), + + /// Error when writing to a file. + #[error("Error when writing to {0}")] + WriteFile(PathBuf, #[source] std::io::Error), /// Pandoc error /// diff --git a/src/templatespec.rs b/src/templatespec.rs index dbb39dc..7b8723b 100644 --- a/src/templatespec.rs +++ b/src/templatespec.rs @@ -40,7 +40,8 @@ impl TemplateSpec { /// Read a template.yaml file and create the corresponding TemplateSpec. pub fn from_file(filename: &Path) -> Result { - let yaml = resource::read_as_string(filename, None)?; + let yaml = resource::read_as_string(filename, None) + .map_err(|err| SubplotError::ReadFile(filename.to_path_buf(), err))?; let spec = TemplateSpec::from_yaml(&yaml)?; let dirname = match filename.parent() { Some(x) => x, -- cgit v1.2.1 From 2a452b2d2f4fba6f840e2ea33123002ae58e88b2 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 6 May 2022 09:13:46 +0300 Subject: refactor: replace generic Pandoc error with specific Replace SubplotError::PandocError with ::Pandoc, which is only for executing the Pandoc program. In reality, so was the old variant, but the new one is more specific, and also embeds the error from executing Pandoc as a source error. The error message is still crap, but that will be fixed later by not using Pandoc for parsing Markdown. Sponsored-by: author --- src/doc.rs | 2 +- src/error.rs | 13 +++---------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/doc.rs b/src/doc.rs index 37f32d6..71910f1 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -181,7 +181,7 @@ impl<'a> Document { crate::policy::add_citeproc(&mut pandoc); trace!("Invoking Pandoc to parse document {:?}", filename); - let output = match pandoc.execute()? { + let output = match pandoc.execute().map_err(SubplotError::Pandoc)? { pandoc::PandocOutput::ToBuffer(o) => o, _ => return Err(SubplotError::NotJson), }; diff --git a/src/error.rs b/src/error.rs index f0d76e3..1b95f1c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -283,16 +283,9 @@ pub enum SubplotError { #[error("Error when writing to {0}")] WriteFile(PathBuf, #[source] std::io::Error), - /// Pandoc error - /// - /// Subplot got an error from Panoc. This is a generic wrapper for - /// any kinds of Pandoc errors. - #[error(transparent)] - PandocError { - /// The wrapped error. - #[from] - source: pandoc::PandocError, - }, + /// Error executing Pandoc. + #[error("Pandoc failed")] + Pandoc(#[source] pandoc::PandocError), /// Regular expression error /// -- cgit v1.2.1 From f42f4b2c27af2041436d681e4e292e549985066c Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 6 May 2022 09:30:27 +0300 Subject: refactor: replace SubplotError::YamlError with more specific one SubplotError::YamlError is quite generic. We only parse YAML as part of document metadata, so replace the error with a more specific one. Sponsored-by: author --- src/bindings.rs | 3 ++- src/error.rs | 13 +++---------- src/templatespec.rs | 2 +- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/bindings.rs b/src/bindings.rs index 5c98297..a6f96ca 100644 --- a/src/bindings.rs +++ b/src/bindings.rs @@ -463,7 +463,8 @@ impl Bindings { /// Add bindings from a YAML string pub fn add_from_yaml(&mut self, yaml: &str) -> Result<(), SubplotError> { - let bindings: Vec = serde_yaml::from_str(yaml)?; + let bindings: Vec = + serde_yaml::from_str(yaml).map_err(SubplotError::Metadata)?; for wrapper in bindings { self.add(from_hashmap(&wrapper.binding)?); } diff --git a/src/error.rs b/src/error.rs index 1b95f1c..5286410 100644 --- a/src/error.rs +++ b/src/error.rs @@ -309,16 +309,9 @@ pub enum SubplotError { source: serde_json::Error, }, - /// YAML error - /// - /// Subplot parses YAML. This is a generic wrapper for any kinds - /// of errors related to that. - #[error(transparent)] - YamlError { - /// The wrapped error. - #[from] - source: serde_yaml::Error, - }, + /// Error parsing YAML metadata for document. + #[error("Failed to parse YAML metadata")] + Metadata(#[source] serde_yaml::Error), /// Abstract syntax tree error. #[error(transparent)] diff --git a/src/templatespec.rs b/src/templatespec.rs index 7b8723b..28ab7e1 100644 --- a/src/templatespec.rs +++ b/src/templatespec.rs @@ -21,7 +21,7 @@ pub struct TemplateSpec { impl TemplateSpec { // Create a new TemplateSpec from YAML text. fn from_yaml(yaml: &str) -> Result { - Ok(serde_yaml::from_str(yaml)?) + serde_yaml::from_str(yaml).map_err(SubplotError::Metadata) } // Create a new TemplateSpec. -- cgit v1.2.1 From 185377775aeeba57f91d6b40a8a599e840db1e90 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 6 May 2022 09:42:14 +0300 Subject: refactor: replace SubplotError::RegexError with a simpler one SubplotError::RegexError was a struct variant in the enum. Replace it with a more usual variant with the regex::Error as a source field, for better error messaging. Sponsored-by: author --- src/bindings.rs | 3 ++- src/error.rs | 8 ++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/bindings.rs b/src/bindings.rs index a6f96ca..5db63c9 100644 --- a/src/bindings.rs +++ b/src/bindings.rs @@ -171,7 +171,8 @@ impl Binding { ) -> Result { let regex = RegexBuilder::new(&format!("^{}$", pattern)) .case_insensitive(!case_sensitive) - .build()?; + .build() + .map_err(|err| SubplotError::Regex(pattern.to_string(), err))?; // For every named capture, ensure we have a known type for it. // If the type is missing from the map, we default to `text` which is // the .* pattern diff --git a/src/error.rs b/src/error.rs index 5286410..8fdaeee 100644 --- a/src/error.rs +++ b/src/error.rs @@ -291,12 +291,8 @@ pub enum SubplotError { /// /// Subplot uses regular expressions. This is a generic wrapper for /// any kinds of errors related to that. - #[error(transparent)] - RegexError { - /// The wrapped error. - #[from] - source: regex::Error, - }, + #[error("Failed to compile regular expression: {0:?}")] + Regex(String, #[source] regex::Error), /// JSON error /// -- cgit v1.2.1 From 52ccfa5a1bf2893bdc63ff1bad2b2a88ca86ef95 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 6 May 2022 09:56:18 +0300 Subject: refactor: replace SubplotError::JsonError with a more specific one SubplotError::JsonError is a generic error, but we actually only parse JSON when parsing the AST from Pandoc. Replace it with a ::AstJson error to be more specific, and hopefully more helpful to the user. Sponsored-by: author --- src/doc.rs | 4 ++-- src/error.rs | 13 +++---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/doc.rs b/src/doc.rs index 71910f1..490a613 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -146,7 +146,7 @@ impl<'a> Document { P: AsRef + Debug, { trace!("Parsing document..."); - let ast: Pandoc = serde_json::from_str(json)?; + let ast: Pandoc = serde_json::from_str(json).map_err(SubplotError::AstJson)?; Self::from_ast(basedir, markdowns, ast, style, template) } @@ -217,7 +217,7 @@ impl<'a> Document { /// This is useful in a Pandoc filter, so that the filter can give /// it back to Pandoc for typesetting. pub fn ast(&self) -> Result { - let json = serde_json::to_string(&self.ast)?; + let json = serde_json::to_string(&self.ast).map_err(SubplotError::AstJson)?; Ok(json) } diff --git a/src/error.rs b/src/error.rs index 8fdaeee..a42298d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -294,16 +294,9 @@ pub enum SubplotError { #[error("Failed to compile regular expression: {0:?}")] Regex(String, #[source] regex::Error), - /// JSON error - /// - /// Subplot parses and generates JSON. This is a generic wrapper - /// for any kinds of errors related to that. - #[error(transparent)] - JsonError { - /// The wrapped error. - #[from] - source: serde_json::Error, - }, + /// Error parsing the Pandoc abstract syntax tree as JSON. + #[error("Failed to parse document AST as JSON")] + AstJson(#[source] serde_json::Error), /// Error parsing YAML metadata for document. #[error("Failed to parse YAML metadata")] -- cgit v1.2.1