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