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/doc.rs | 56 +++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 21 deletions(-) (limited to 'src/doc.rs') 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() { -- 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/doc.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/doc.rs') 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"); -- 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 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/doc.rs') 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), }; -- 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 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/doc.rs') 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) } -- cgit v1.2.1