From 19f6391d8a6033c7a415d87cb5aa13daef67196a Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:41:17 +0200 Subject: [PATCH 01/22] Implement Error for ExitCode Signed-off-by: Matthias Beyer --- lib/core/libimagerror/src/exit.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lib/core/libimagerror/src/exit.rs b/lib/core/libimagerror/src/exit.rs index 40ac9b9f..85ae5466 100644 --- a/lib/core/libimagerror/src/exit.rs +++ b/lib/core/libimagerror/src/exit.rs @@ -17,6 +17,10 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // +use std::error::Error; +use std::fmt::{Formatter, Display}; + +#[derive(Debug)] pub struct ExitCode(i32); impl From for ExitCode { @@ -31,6 +35,26 @@ impl ExitCode { } } +impl Display for ExitCode { + fn fmt(&self, f: &mut Formatter) -> Result<(), std::fmt::Error> { + write!(f, "ExitCode {}", self.0) + } +} + +impl Error for ExitCode { + fn description(&self) -> &str { + "ExitCode" + } + + fn cause(&self) -> Option<&dyn Error> { + None + } + + fn source(&self) -> Option<&(dyn Error + 'static)> { + None + } +} + pub trait ExitUnwrap { fn unwrap_or_exit(self) -> T; } From e259015a611aadc5de8973c15c9f6ce99febf38b Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 02/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag/Cargo.toml | 2 +- bin/core/imag/src/main.rs | 336 ++++++++++++++++---------------------- 2 files changed, 143 insertions(+), 195 deletions(-) diff --git a/bin/core/imag/Cargo.toml b/bin/core/imag/Cargo.toml index 4c7df6dc..1250b2f7 100644 --- a/bin/core/imag/Cargo.toml +++ b/bin/core/imag/Cargo.toml @@ -20,7 +20,6 @@ libimagerror = { version = "0.10.0", path = "../../../lib/core/libimagerror" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagentrytag = { version = "0.10.0", path = "../../../lib/entry/libimagentrytag" } libimagutil = { version = "0.10.0", path = "../../../lib/etc/libimagutil" } -failure = "0.1.5" log = "0.4.6" # Build time dependencies for cli completion @@ -61,6 +60,7 @@ walkdir = "2.2.8" log = "0.4.6" toml = "0.5.1" toml-query = "0.9.2" +failure = "0.1.5" libimagerror = { version = "0.10.0", path = "../../../lib/core/libimagerror" } libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } diff --git a/bin/core/imag/src/main.rs b/bin/core/imag/src/main.rs index 22ecbaa4..d879d755 100644 --- a/bin/core/imag/src/main.rs +++ b/bin/core/imag/src/main.rs @@ -36,6 +36,7 @@ extern crate clap; #[macro_use] extern crate log; +#[macro_use] extern crate failure; extern crate walkdir; extern crate toml; extern crate toml_query; @@ -44,11 +45,10 @@ extern crate toml_query; extern crate libimagerror; use std::env; -use std::process::exit; use std::process::Command; use std::process::Stdio; use std::io::ErrorKind; -use std::io::{stdout, Stdout, Write}; +use std::io::{stdout, Write}; use std::collections::BTreeMap; use std::path::PathBuf; @@ -56,12 +56,13 @@ use walkdir::WalkDir; use clap::{Arg, ArgMatches, AppSettings, SubCommand}; use toml::Value; use toml_query::read::TomlValueReadExt; +use failure::Error; +use failure::ResultExt; +use failure::err_msg; +use failure::Fallible as Result; use libimagrt::runtime::Runtime; use libimagrt::spec::CliSpec; -use libimagerror::io::ToExitCode; -use libimagerror::exit::ExitUnwrap; -use libimagerror::trace::trace_error; use libimagrt::configuration::InternalConfiguration; /// Returns the helptext, putting the Strings in cmds as possible @@ -107,54 +108,44 @@ fn help_text(cmds: Vec) -> String { } /// Returns the list of imag-* executables found in $PATH -fn get_commands(out: &mut Stdout) -> Vec { - let mut v = match env::var("PATH") { - Err(e) => { - writeln!(out, "PATH error: {:?}", e) - .to_exit_code() - .unwrap_or_exit(); - exit(1) - }, - - Ok(path) => path - .split(':') - .flat_map(|elem| { - WalkDir::new(elem) - .max_depth(1) - .into_iter() - .filter(|path| match *path { - Ok(ref p) => p.file_name().to_str().map_or(false, |f| f.starts_with("imag-")), - Err(_) => false, - }) - .filter_map(Result::ok) - .filter_map(|path| path - .file_name() - .to_str() - .and_then(|s| s.splitn(2, '-').nth(1).map(String::from)) - ) - }) - .filter(|path| if cfg!(debug_assertions) { - // if we compile in debug mode during development, ignore everything that ends with - // ".d", as developers might use the ./target/debug/ directory directly in `$PATH`. - !path.ends_with(".d") - } else { - true - }) - .collect::>() - }; +fn get_commands() -> Result> { + let mut v = env::var("PATH")? + .split(':') + .flat_map(|elem| { + WalkDir::new(elem) + .max_depth(1) + .into_iter() + .filter(|path| match *path { + Ok(ref p) => p.file_name().to_str().map_or(false, |f| f.starts_with("imag-")), + Err(_) => false, + }) + .filter_map(|r| r.ok()) + .filter_map(|path| path + .file_name() + .to_str() + .and_then(|s| s.splitn(2, '-').nth(1).map(String::from)) + ) + }) + .filter(|path| if cfg!(debug_assertions) { + // if we compile in debug mode during development, ignore everything that ends with + // ".d", as developers might use the ./target/debug/ directory directly in `$PATH`. + !path.ends_with(".d") + } else { + true + }) + .collect::>(); v.sort(); - v + Ok(v) } -fn main() { +fn main() -> Result<()> { // Initialize the Runtime and build the CLI let appname = "imag"; let version = make_imag_version!(); let about = "imag - the PIM suite for the commandline"; - let mut out = stdout(); - let commands = get_commands(&mut out); + let commands = get_commands()?; let helptext = help_text(commands.clone()); let mut app = Runtime::get_default_cli_builder(appname, &version, about) .settings(&[AppSettings::AllowExternalSubcommands, AppSettings::ArgRequiredElseHelp]) @@ -175,174 +166,131 @@ fn main() { let long_help = { let mut v = vec![]; - if let Err(e) = app.write_long_help(&mut v) { - eprintln!("Error: {:?}", e); - exit(1); - } - String::from_utf8(v).unwrap_or_else(|_| { eprintln!("UTF8 Error"); exit(1) }) + app.write_long_help(&mut v)?; + String::from_utf8(v).map_err(|_| err_msg("UTF8 Error"))? }; - { - let print_help = app.clone().get_matches().subcommand_name().map(|h| h == "help").unwrap_or(false); - if print_help { - writeln!(out, "{}", long_help) - .to_exit_code() - .unwrap_or_exit(); - exit(0) + let print_help = app.clone().get_matches().subcommand_name().map(|h| h == "help").unwrap_or(false); + + let mut out = stdout(); + if print_help { + writeln!(out, "{}", long_help).map_err(Error::from) + } else { + let enable_logging = app.enable_logging(); + let matches = app.matches(); + + let rtp = ::libimagrt::runtime::get_rtp_match(&matches)?; + let configpath = matches + .value_of("config") + .map_or_else(|| rtp.clone(), PathBuf::from); + debug!("Config path = {:?}", configpath); + let config = ::libimagrt::configuration::fetch_config(&configpath)?; + + if enable_logging { + Runtime::init_logger(&matches, config.as_ref()) } - } - let enable_logging = app.enable_logging(); - let matches = app.matches(); + debug!("matches: {:?}", matches); - let rtp = ::libimagrt::runtime::get_rtp_match(&matches) - .unwrap_or_else(|e| { - trace_error(&e); - exit(1) - }); - let configpath = matches - .value_of("config") - .map_or_else(|| rtp.clone(), PathBuf::from); - debug!("Config path = {:?}", configpath); - let config = ::libimagrt::configuration::fetch_config(&configpath) - .unwrap_or_else(|e| { - trace_error(&e); - exit(1) - }); + // Begin checking for arguments - if enable_logging { - Runtime::init_logger(&matches, config.as_ref()) - } + if matches.is_present("version") { + debug!("Showing version"); + writeln!(out, "imag {}", env!("CARGO_PKG_VERSION")).map_err(Error::from) + } else { + if matches.is_present("versions") { + debug!("Showing versions"); + commands + .iter() + .map(|command| { + match Command::new(format!("imag-{}", command)) + .stdin(::std::process::Stdio::inherit()) + .stdout(::std::process::Stdio::piped()) + .stderr(::std::process::Stdio::inherit()) + .arg("--version") + .output() + .map(|v| v.stdout) + { + Ok(s) => match String::from_utf8(s) { + Ok(s) => format!("{:15} -> {}", command, s), + Err(e) => format!("UTF8 Error while working with output of imag{}: {:?}", command, e), + }, + Err(e) => format!("Failed calling imag-{} -> {:?}", command, e), + } + }) + .fold(Ok(()), |_, line| { + // The amount of newlines may differ depending on the subprocess + writeln!(out, "{}", line.trim()).map_err(Error::from) + }) + } else { + let aliases = fetch_aliases(config.as_ref()) + .map_err(Error::from) + .context("Error while fetching aliases from configuration file")?; - debug!("matches: {:?}", matches); + // Matches any subcommand given, except calling for example 'imag --versions', as this option + // does not exit. There's nothing to do in such a case + if let (subcommand, Some(scmd)) = matches.subcommand() { + // Get all given arguments and further subcommands to pass to + // the imag-<> binary + // Providing no arguments is OK, and is therefore ignored here + let mut subcommand_args : Vec = match scmd.values_of("") { + Some(values) => values.map(String::from).collect(), + None => Vec::new() + }; - // Begin checking for arguments + debug!("Processing forwarding of commandline arguments"); + forward_commandline_arguments(&matches, &mut subcommand_args); - if matches.is_present("version") { - debug!("Showing version"); - writeln!(out, "imag {}", env!("CARGO_PKG_VERSION")) - .to_exit_code() - .unwrap_or_exit(); - exit(0); - } + let subcommand = String::from(subcommand); + let subcommand = aliases.get(&subcommand).cloned().unwrap_or(subcommand); - if matches.is_present("versions") { - debug!("Showing versions"); - commands - .iter() - .map(|command| { - match Command::new(format!("imag-{}", command)) - .stdin(::std::process::Stdio::inherit()) - .stdout(::std::process::Stdio::piped()) - .stderr(::std::process::Stdio::inherit()) - .arg("--version") - .output() - .map(|v| v.stdout) - { - Ok(s) => match String::from_utf8(s) { - Ok(s) => format!("{:15} -> {}", command, s), - Err(e) => format!("UTF8 Error while working with output of imag{}: {:?}", command, e), - }, - Err(e) => format!("Failed calling imag-{} -> {:?}", command, e), - } - }) - .fold((), |_, line| { - // The amount of newlines may differ depending on the subprocess - writeln!(out, "{}", line.trim()) - .to_exit_code() - .unwrap_or_exit(); - }); + debug!("Calling 'imag-{}' with args: {:?}", subcommand, subcommand_args); - exit(0); - } + // Create a Command, and pass it the gathered arguments + match Command::new(format!("imag-{}", subcommand)) + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .args(&subcommand_args[..]) + .spawn() + .and_then(|mut c| c.wait()) + { + Ok(exit_status) => if !exit_status.success() { + debug!("imag-{} exited with non-zero exit code: {:?}", subcommand, exit_status); + Err(format_err!("imag-{} exited with non-zero exit code", subcommand)) + } else { + debug!("Successful exit!"); + Ok(()) + }, - let aliases = match fetch_aliases(config.as_ref()) { - Ok(aliases) => aliases, - Err(e) => { - writeln!(out, "Error while fetching aliases from configuration file") - .to_exit_code() - .unwrap_or_exit(); - debug!("Error = {:?}", e); - writeln!(out, "Aborting") - .to_exit_code() - .unwrap_or_exit(); - exit(1); - } - }; - - // Matches any subcommand given, except calling for example 'imag --versions', as this option - // does not exit. There's nothing to do in such a case - if let (subcommand, Some(scmd)) = matches.subcommand() { - // Get all given arguments and further subcommands to pass to - // the imag-<> binary - // Providing no arguments is OK, and is therefore ignored here - let mut subcommand_args : Vec = match scmd.values_of("") { - Some(values) => values.map(String::from).collect(), - None => Vec::new() - }; - - debug!("Processing forwarding of commandline arguments"); - forward_commandline_arguments(&matches, &mut subcommand_args); - - let subcommand = String::from(subcommand); - let subcommand = aliases.get(&subcommand).cloned().unwrap_or(subcommand); - - debug!("Calling 'imag-{}' with args: {:?}", subcommand, subcommand_args); - - // Create a Command, and pass it the gathered arguments - match Command::new(format!("imag-{}", subcommand)) - .stdin(Stdio::inherit()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .args(&subcommand_args[..]) - .spawn() - .and_then(|mut c| c.wait()) - { - Ok(exit_status) => { - if !exit_status.success() { - debug!("imag-{} exited with non-zero exit code: {:?}", subcommand, exit_status); - eprintln!("imag-{} exited with non-zero exit code", subcommand); - exit(exit_status.code().unwrap_or(1)); - } - debug!("Successful exit!"); - }, - - Err(e) => { - debug!("Error calling the subcommand"); - match e.kind() { - ErrorKind::NotFound => { - writeln!(out, "No such command: 'imag-{}'", subcommand) - .to_exit_code() - .unwrap_or_exit(); - writeln!(out, "See 'imag --help' for available subcommands") - .to_exit_code() - .unwrap_or_exit(); - exit(1); - }, - ErrorKind::PermissionDenied => { - writeln!(out, "No permission to execute: 'imag-{}'", subcommand) - .to_exit_code() - .unwrap_or_exit(); - exit(1); - }, - _ => { - writeln!(out, "Error spawning: {:?}", e) - .to_exit_code() - .unwrap_or_exit(); - exit(1); + Err(e) => { + debug!("Error calling the subcommand"); + match e.kind() { + ErrorKind::NotFound => { + writeln!(out, "No such command: 'imag-{}'", subcommand)?; + writeln!(out, "See 'imag --help' for available subcommands").map_err(Error::from) + }, + ErrorKind::PermissionDenied => { + writeln!(out, "No permission to execute: 'imag-{}'", subcommand).map_err(Error::from) + }, + _ => writeln!(out, "Error spawning: {:?}", e).map_err(Error::from), + } + } } + } else { + Ok(()) } } } } } -fn fetch_aliases(config: Option<&Value>) -> Result, String> { - let cfg = config.ok_or_else(|| String::from("No configuration found"))?; +fn fetch_aliases(config: Option<&Value>) -> Result> { + let cfg = config.ok_or_else(|| err_msg("No configuration found"))?; let value = cfg .read("imag.aliases") - .map_err(|_| String::from("Reading from config failed")); + .map_err(|_| err_msg("Reading from config failed"))?; - match value? { + match value { None => Ok(BTreeMap::new()), Some(&Value::Table(ref tbl)) => { let mut alias_mappings = BTreeMap::new(); @@ -359,7 +307,7 @@ fn fetch_aliases(config: Option<&Value>) -> Result, Str alias_mappings.insert(s.clone(), k.clone()); }, _ => { - let e = format!("Not all values are a String in 'imag.aliases.{}'", k); + let e = format_err!("Not all values are a String in 'imag.aliases.{}'", k); return Err(e); } } @@ -367,7 +315,7 @@ fn fetch_aliases(config: Option<&Value>) -> Result, Str }, _ => { - let msg = format!("Type Error: 'imag.aliases.{}' is not a table or string", k); + let msg = format_err!("Type Error: 'imag.aliases.{}' is not a table or string", k); return Err(msg); }, } @@ -376,7 +324,7 @@ fn fetch_aliases(config: Option<&Value>) -> Result, Str Ok(alias_mappings) }, - Some(_) => Err(String::from("Type Error: 'imag.aliases' is not a table")), + Some(_) => Err(err_msg("Type Error: 'imag.aliases' is not a table")), } } From b36454ac3834f46cf47131e404a07a40ccf3c6d8 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Wed, 23 Oct 2019 19:04:19 +0200 Subject: [PATCH 03/22] Refactor to not call exit() anywhere Signed-off-by: Matthias Beyer --- bin/core/imag-annotate/src/lib.rs | 193 ++++++++++++------------------ 1 file changed, 78 insertions(+), 115 deletions(-) diff --git a/bin/core/imag-annotate/src/lib.rs b/bin/core/imag-annotate/src/lib.rs index e2501985..0e80fbe5 100644 --- a/bin/core/imag-annotate/src/lib.rs +++ b/bin/core/imag-annotate/src/lib.rs @@ -53,17 +53,15 @@ use std::io::Write; use failure::Error; use failure::Fallible as Result; +use failure::ResultExt; +use failure::err_msg; use toml_query::read::TomlValueReadTypeExt; use clap::App; use libimagentryannotation::annotateable::*; use libimagentryannotation::annotation_fetcher::*; use libimagentryedit::edit::*; -use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; use libimagerror::errors::ErrorMsg as EM; -use libimagerror::iter::TraceIterator; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; use libimagstore::store::FileLockEntry; @@ -75,22 +73,19 @@ mod ui; pub enum ImagAnnotate {} impl ImagApplication for ImagAnnotate { fn run(rt: Runtime) -> Result<()> { - if let Some(name) = rt.cli().subcommand_name() { - match name { - "add" => add(&rt), - "remove" => remove(&rt), - "list" => list(&rt), - other => { - debug!("Unknown command"); - let _ = rt.handle_unknown_subcommand("imag-annotation", other, rt.cli()) - .map_err_trace_exit_unwrap() - .code() - .map(::std::process::exit); - }, - } + match rt.cli().subcommand_name().ok_or_else(|| err_msg("No command called"))? { + "add" => add(&rt), + "remove" => remove(&rt), + "list" => list(&rt), + other => { + debug!("Unknown command"); + if rt.handle_unknown_subcommand("imag-annotation", other, rt.cli())?.success() { + Ok(()) + } else { + Err(err_msg("Failed to handle unknown subcommand")) + } + }, } - - Ok(()) } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { @@ -110,149 +105,119 @@ impl ImagApplication for ImagAnnotate { } } -fn add(rt: &Runtime) { +fn add(rt: &Runtime) -> Result<()> { let scmd = rt.cli().subcommand_matches("add").unwrap(); // safed by main() let mut ids = rt .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No StoreId supplied"); - ::std::process::exit(1); - }) + .context("No StoreId supplied")? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter(); if let Some(first) = ids.next() { let mut annotation = rt.store() - .get(first.clone()) - .map_err_trace_exit_unwrap() - .ok_or_else(|| EM::EntryNotFound(first.local_display_string())) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - .annotate(rt.store()) - .map_err_trace_exit_unwrap(); + .get(first.clone())? + .ok_or_else(|| EM::EntryNotFound(first.local_display_string()))? + .annotate(rt.store())?; - annotation.edit_content(&rt).map_err_trace_exit_unwrap(); + annotation.edit_content(&rt)?; for id in ids { - let mut entry = rt.store().get(id.clone()) - .map_err_trace_exit_unwrap() - .ok_or_else(|| format_err!("Not found: {}", id.local_display_string())) - .map_err_trace_exit_unwrap(); + let mut entry = rt.store().get(id.clone())? + .ok_or_else(|| format_err!("Not found: {}", id.local_display_string()))?; - entry.add_link(&mut annotation).map_err_trace_exit_unwrap(); + entry.add_link(&mut annotation)?; } if !scmd.is_present("dont-print-name") { if let Some(annotation_id) = annotation .get_header() - .read_string("annotation.name") - .map_err(Error::from) - .map_err_trace_exit_unwrap() + .read_string("annotation.name")? { - writeln!(rt.stdout(), "Name of the annotation: {}", annotation_id) - .to_exit_code() - .unwrap_or_exit(); + writeln!(rt.stdout(), "Name of the annotation: {}", annotation_id)?; } else { - error!("Unnamed annotation: {:?}", annotation.get_location()); - error!("This is most likely a BUG, please report!"); + Err(format_err!("Unnamed annotation: {:?}", annotation.get_location())) + .context("This is most likely a BUG, please report!")?; } } } else { debug!("No entries to annotate"); } + + Ok(()) } -fn remove(rt: &Runtime) { - let scmd = rt.cli().subcommand_matches("remove").unwrap(); // safed by main() +fn remove(rt: &Runtime) -> Result<()> { + let scmd = rt.cli().subcommand_matches("remove").unwrap(); // safed by main() let annotation_name = scmd.value_of("annotation_name").unwrap(); // safed by clap - let delete = scmd.is_present("delete-annotation"); - let ids = rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) - .into_iter(); + let delete = scmd.is_present("delete-annotation"); - ids.for_each(|id| { - let mut entry = rt.store() - .get(id.clone()) - .map_err_trace_exit_unwrap() - .ok_or_else(|| EM::EntryNotFound(id.local_display_string())) - .map_err(Error::from) - .map_err_trace_exit_unwrap(); + rt.ids::() + .context("No ids supplied")? + .ok_or_else(|| err_msg("No ids supplied"))? + .into_iter() + .map(|id| { + let mut entry = rt.store() + .get(id.clone())? + .ok_or_else(|| EM::EntryNotFound(id.local_display_string()))?; - let annotation = entry - .denotate(rt.store(), annotation_name) - .map_err_trace_exit_unwrap(); + let annotation = entry.denotate(rt.store(), annotation_name)?; - if delete { - debug!("Deleting annotation object"); - if let Some(an) = annotation { - let loc = an.get_location().clone(); - drop(an); + if delete { + debug!("Deleting annotation object"); + if let Some(an) = annotation { + let loc = an.get_location().clone(); + drop(an); - rt - .store() - .delete(loc) - .map_err_trace_exit_unwrap(); + rt.store().delete(loc)?; + } else { + warn!("Not having annotation object, cannot delete!"); + } } else { - warn!("Not having annotation object, cannot delete!"); + debug!("Not deleting annotation object"); } - } else { - debug!("Not deleting annotation object"); - } - }) + Ok(()) + }) + .collect() } -fn list(rt: &Runtime) { - let scmd = rt.cli().subcommand_matches("list").unwrap(); // safed by clap +fn list(rt: &Runtime) -> Result<()> { + let scmd = rt.cli().subcommand_matches("list").unwrap(); // safed by clap let with_text = scmd.is_present("list-with-text"); let ids = rt .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) - .into_iter(); + .context("No ids supplied")? + .ok_or_else(|| err_msg("No ids supplied"))?; if ids.len() != 0 { - ids - .for_each(|id| { - rt - .store() - .get(id.clone()) - .map_err_trace_exit_unwrap() - .ok_or_else(|| EM::EntryNotFound(id.local_display_string())) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - .annotations() - .map_err_trace_exit_unwrap() + ids.into_iter() + .map(|id| -> Result<_> { + let lds = id.local_display_string(); + Ok(rt.store() + .get(id)? + .ok_or_else(|| EM::EntryNotFound(lds))? + .annotations()? .into_get_iter(rt.store()) - .trace_unwrap_exit() - .map(|opt| opt.ok_or_else(|| format_err!("Cannot find entry"))) - .trace_unwrap_exit() + .map(|el| el.and_then(|o| o.ok_or_else(|| format_err!("Cannot find entry")))) .enumerate() - .for_each(|(i, entry)| list_annotation(&rt, i, entry, with_text)); - }); + .map(|(i, entry)| entry.and_then(|e| list_annotation(&rt, i, e, with_text))) + .collect()) + }) + .flatten() + .collect() } else { // ids.len() == 0 // show them all rt.store() - .all_annotations() - .map_err_trace_exit_unwrap() + .all_annotations()? .into_get_iter() - .trace_unwrap_exit() - .map(|opt| opt.ok_or_else(|| format_err!("Cannot find entry"))) - .trace_unwrap_exit() + .map(|el| el.and_then(|opt| opt.ok_or_else(|| format_err!("Cannot find entry")))) .enumerate() - .for_each(|(i, entry)| list_annotation(&rt, i, entry, with_text)); + .map(|(i, entry)| entry.and_then(|e| list_annotation(&rt, i, e, with_text))) + .collect() } } -fn list_annotation<'a>(rt: &Runtime, i: usize, a: FileLockEntry<'a>, with_text: bool) { +fn list_annotation<'a>(rt: &Runtime, i: usize, a: FileLockEntry<'a>, with_text: bool) -> Result<()> { if with_text { writeln!(rt.stdout(), "--- {i: >5} | {id}\n{text}\n\n", @@ -261,8 +226,6 @@ fn list_annotation<'a>(rt: &Runtime, i: usize, a: FileLockEntry<'a>, with_text: text = a.get_content()) } else { writeln!(rt.stdout(), "{: >5} | {}", i, a.get_location()) - } - .to_exit_code() - .unwrap_or_exit(); + }.map_err(Error::from) } From 647ca2fea607d295a83ecd8a94f7488a2fbedbd0 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 04/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-category/Cargo.toml | 1 + bin/core/imag-category/src/lib.rs | 150 +++++++++++------------------- 2 files changed, 55 insertions(+), 96 deletions(-) diff --git a/bin/core/imag-category/Cargo.toml b/bin/core/imag-category/Cargo.toml index afc09fbb..1333f8f8 100644 --- a/bin/core/imag-category/Cargo.toml +++ b/bin/core/imag-category/Cargo.toml @@ -24,6 +24,7 @@ log = "0.4.6" toml = "0.5.1" toml-query = "0.9.2" failure = "0.1.5" +resiter = "0.3.0" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-category/src/lib.rs b/bin/core/imag-category/src/lib.rs index a40bd632..07034d10 100644 --- a/bin/core/imag-category/src/lib.rs +++ b/bin/core/imag-category/src/lib.rs @@ -39,6 +39,7 @@ extern crate clap; extern crate log; #[macro_use] extern crate failure; +extern crate resiter; extern crate libimagentrycategory; extern crate libimagerror; @@ -50,19 +51,20 @@ use failure::Fallible as Result; use clap::App; use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; +use libimagerror::iter::IterInnerOkOrElse; mod ui; use std::io::Write; +use failure::err_msg; +use failure::Error; +use resiter::AndThen; + use libimagentrycategory::store::CategoryStore; -use libimagstore::storeid::StoreIdIterator; use libimagstore::iter::get::StoreIdGetIteratorExtension; -use libimagerror::iter::TraceIterator; use libimagentrycategory::entry::EntryCategory; use libimagentrycategory::category::Category; @@ -73,25 +75,22 @@ use libimagentrycategory::category::Category; pub enum ImagCategory {} impl ImagApplication for ImagCategory { fn run(rt: Runtime) -> Result<()> { - if let Some(name) = rt.cli().subcommand_name() { - match name { - "set" => set(&rt), - "get" => get(&rt), - "list-category" => list_category(&rt), - "create-category" => create_category(&rt), - "delete-category" => delete_category(&rt), - "list-categories" => list_categories(&rt), - other => { - debug!("Unknown command"); - let _ = rt.handle_unknown_subcommand("imag-category", other, rt.cli()) - .map_err_trace_exit_unwrap() - .code() - .map(::std::process::exit); - }, - } + match rt.cli().subcommand_name().ok_or_else(|| err_msg("No subcommand called"))? { + "set" => set(&rt), + "get" => get(&rt), + "list-category" => list_category(&rt), + "create-category" => create_category(&rt), + "delete-category" => delete_category(&rt), + "list-categories" => list_categories(&rt), + other => { + debug!("Unknown command"); + if rt.handle_unknown_subcommand("imag-category", other, rt.cli())?.success() { + Ok(()) + } else { + Err(err_msg("Failed to handle unknown subcommand")) + } + }, } - - Ok(()) } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { @@ -112,125 +111,84 @@ impl ImagApplication for ImagCategory { } -fn set(rt: &Runtime) { +fn set(rt: &Runtime) -> Result<()> { let scmd = rt.cli().subcommand_matches("set").unwrap(); // safed by main() let name = scmd.value_of("set-name").map(String::from).unwrap(); // safed by clap - let sids = rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) - .into_iter(); - - StoreIdIterator::new(Box::new(sids.map(Ok))) + rt.ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? + .into_iter() + .map(Ok) .into_get_iter(rt.store()) - .trace_unwrap_exit() - .map(|o| o.unwrap_or_else(|| { - error!("Did not find one entry"); - ::std::process::exit(1) - })) - .for_each(|mut entry| { - entry - .set_category_checked(rt.store(), &name) - .map_err_trace_exit_unwrap(); - }) + .map_inner_ok_or_else(|| err_msg("Did not find one entry")) + .and_then_ok(|mut e| e.set_category_checked(rt.store(), &name)) + .collect() } -fn get(rt: &Runtime) { +fn get(rt: &Runtime) -> Result<()> { let out = rt.stdout(); let mut outlock = out.lock(); - let sids = rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) - .into_iter(); - - StoreIdIterator::new(Box::new(sids.map(Ok))) + rt.ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? + .into_iter() + .map(Ok) .into_get_iter(rt.store()) - .trace_unwrap_exit() - .map(|o| o.unwrap_or_else(|| { - error!("Did not find one entry"); - ::std::process::exit(1) - })) - .map(|entry| entry.get_category().map_err_trace_exit_unwrap()) - .for_each(|name| { - writeln!(outlock, "{}", name).to_exit_code().unwrap_or_exit(); - }) + .map(|el| el.and_then(|o| o.ok_or_else(|| err_msg("Did not find one entry")))) + .map(|entry| entry.and_then(|e| e.get_category())) + .map(|name| name.and_then(|n| writeln!(outlock, "{}", n).map_err(Error::from))) + .collect() } -fn list_category(rt: &Runtime) { +fn list_category(rt: &Runtime) -> Result<()> { let scmd = rt.cli().subcommand_matches("list-category").unwrap(); // safed by main() let name = scmd.value_of("list-category-name").map(String::from).unwrap(); // safed by clap - if let Some(category) = rt.store().get_category_by_name(&name).map_err_trace_exit_unwrap() { + if let Some(category) = rt.store().get_category_by_name(&name)? { let out = rt.stdout(); let mut outlock = out.lock(); category .get_entries(rt.store()) .map_err_trace_exit_unwrap() - .for_each(|entry| { - writeln!(outlock, "{}", entry.map_err_trace_exit_unwrap().get_location()) - .to_exit_code() - .unwrap_or_exit(); - }) + .map(|entry| writeln!(outlock, "{}", entry?.get_location()).map_err(Error::from)) + .collect() } else { - info!("No category named '{}'", name); - ::std::process::exit(1) + Err(format_err!("No category named '{}'", name)) } } -fn create_category(rt: &Runtime) { +fn create_category(rt: &Runtime) -> Result<()> { let scmd = rt.cli().subcommand_matches("create-category").unwrap(); // safed by main() let name = scmd.value_of("create-category-name").map(String::from).unwrap(); // safed by clap - - let _ = rt - .store() - .create_category(&name) - .map_err_trace_exit_unwrap(); + rt.store().create_category(&name).map(|_| ()) } -fn delete_category(rt: &Runtime) { +fn delete_category(rt: &Runtime) -> Result<()> { use libimaginteraction::ask::ask_bool; let scmd = rt.cli().subcommand_matches("delete-category").unwrap(); // safed by main() let name = scmd.value_of("delete-category-name").map(String::from).unwrap(); // safed by clap let ques = format!("Do you really want to delete category '{}' and remove links to all categorized enties?", name); - let mut input = rt.stdin().unwrap_or_else(|| { - error!("No input stream. Cannot ask for permission"); - ::std::process::exit(1) - }); + let mut input = rt.stdin().ok_or_else(|| err_msg("No input stream. Cannot ask for permission"))?; let mut output = rt.stdout(); - let answer = ask_bool(&ques, Some(false), &mut input, &mut output).map_err_trace_exit_unwrap(); + let answer = ask_bool(&ques, Some(false), &mut input, &mut output)?; if answer { info!("Deleting category '{}'", name); - rt - .store() - .delete_category(&name) - .map_err_trace_exit_unwrap(); + rt.store().delete_category(&name).map(|_| ()) } else { info!("Not doing anything"); + Ok(()) } } -fn list_categories(rt: &Runtime) { +fn list_categories(rt: &Runtime) -> Result<()> { let out = rt.stdout(); let mut outlock = out.lock(); rt.store() - .all_category_names() - .map_err_trace_exit_unwrap() - .for_each(|name| { - writeln!(outlock, "{}", name.map_err_trace_exit_unwrap()) - .to_exit_code() - .unwrap_or_exit(); - }) + .all_category_names()? + .map(|name| name.and_then(|n| writeln!(outlock, "{}", n).map_err(Error::from))) + .collect() } From 6ab5d1d880be5e4a0b596f3954e8b5ecf01dde25 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 05/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-create/src/lib.rs | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/bin/core/imag-create/src/lib.rs b/bin/core/imag-create/src/lib.rs index 9e7b607a..ced6e0ac 100644 --- a/bin/core/imag-create/src/lib.rs +++ b/bin/core/imag-create/src/lib.rs @@ -36,21 +36,19 @@ extern crate clap; extern crate failure; -#[macro_use] extern crate log; extern crate libimagerror; extern crate libimagrt; extern crate libimagstore; use failure::Fallible as Result; +use failure::err_msg; use clap::App; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; -use libimagerror::trace::MapErrTrace; use libimagstore::iter::create::StoreIdCreateIteratorExtension; use libimagstore::iter::retrieve::StoreIdRetrieveIteratorExtension; -use libimagerror::exit::ExitUnwrap; mod ui; @@ -60,30 +58,17 @@ pub enum ImagCreate {} impl ImagApplication for ImagCreate { fn run(rt: Runtime) -> Result<()> { let force = rt.cli().is_present("force"); - debug!("Detected force = {}", force); - let ids = rt.ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + let ids = rt.ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter() - .map(|id| { debug!("id = {}", id); id }) .map(Ok); if force { ids.into_retrieve_iter(rt.store()).collect::>>() } else { ids.into_create_iter(rt.store()).collect::>>() - }.map_err_trace_exit_unwrap() - .into_iter() - .for_each(|el| { - rt.report_touched(el.get_location()).unwrap_or_exit(); - trace!("Entry = {}", el.get_location()); - }); - - Ok(()) + }.map(|_| ()) } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { From e7e5d3064585cb5ea0bb6f1ca36df592c2d41949 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 06/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-diagnostics/Cargo.toml | 1 + bin/core/imag-diagnostics/src/lib.rs | 91 ++++++++++------------------ 2 files changed, 33 insertions(+), 59 deletions(-) diff --git a/bin/core/imag-diagnostics/Cargo.toml b/bin/core/imag-diagnostics/Cargo.toml index d0c1b102..efae55fc 100644 --- a/bin/core/imag-diagnostics/Cargo.toml +++ b/bin/core/imag-diagnostics/Cargo.toml @@ -19,6 +19,7 @@ toml = "0.5.1" toml-query = "0.9.2" indicatif = "0.12.0" failure = "0.1.5" +resiter = "0.3.0" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-diagnostics/src/lib.rs b/bin/core/imag-diagnostics/src/lib.rs index 1118630f..aeec93d6 100644 --- a/bin/core/imag-diagnostics/src/lib.rs +++ b/bin/core/imag-diagnostics/src/lib.rs @@ -39,6 +39,7 @@ extern crate toml; extern crate toml_query; extern crate indicatif; extern crate failure; +extern crate resiter; #[macro_use] extern crate log; extern crate libimagrt; @@ -50,20 +51,18 @@ use std::io::Write; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; -use libimagerror::trace::MapErrTrace; -use libimagerror::io::ToExitCode; -use libimagerror::exit::ExitUnwrap; use libimagstore::store::FileLockEntry; use libimagstore::storeid::StoreId; use libimagentrylink::linkable::Linkable; +use libimagerror::iter::IterInnerOkOrElse; use toml::Value; use toml_query::read::TomlValueReadExt; use indicatif::{ProgressBar, ProgressStyle}; use failure::Fallible as Result; -use failure::Error; use failure::err_msg; use clap::App; +use resiter::AndThen; use std::collections::BTreeMap; @@ -106,20 +105,6 @@ impl Diagnostic { } } -macro_rules! do_write { - ($dest:ident, $pattern:tt) => { - let _ = writeln!($dest, $pattern) - .to_exit_code() - .unwrap_or_exit(); - }; - - ($dest:ident, $pattern:tt, $( $args:expr ),*) => { - let _ = writeln!($dest, $pattern, $( $args ),*) - .to_exit_code() - .unwrap_or_exit(); - } -} - /// Marker enum for implementing ImagApplication on /// /// This is used by binaries crates to execute business logic @@ -127,8 +112,8 @@ macro_rules! do_write { pub enum ImagDiagnostics {} impl ImagApplication for ImagDiagnostics { fn run(rt: Runtime) -> Result<()> { - let template = get_config(&rt, "rt.progressbar_style"); - let tick_chars = get_config(&rt, "rt.progressticker_chars"); + let template = get_config(&rt, "rt.progressbar_style")?; + let tick_chars = get_config(&rt, "rt.progressticker_chars")?; let verbose = rt.cli().is_present("more-output"); let style = if let Some(tick_chars) = tick_chars { @@ -143,22 +128,16 @@ impl ImagApplication for ImagDiagnostics { spinner.set_message("Accumulating data"); let diags = rt.store() - .entries() - .map_err_trace_exit_unwrap() + .entries()? .into_get_iter() - .map(|e| { - e.map_err_trace_exit_unwrap() - .ok_or_else(|| Error::from(err_msg("Unable to get entry".to_owned()))) - .map_err_trace_exit_unwrap() - }) - .map(|e| { + .map_inner_ok_or_else(|| err_msg("Unable to get entry")) + .and_then_ok(|e| { let diag = Diagnostic::for_entry(&e); debug!("Diagnostic for '{:?}' = {:?}", e.get_location(), diag); drop(e); diag }) - .collect::>>() - .map_err_trace_exit_unwrap(); + .collect::>>()?; spinner.finish(); let n = diags.len(); @@ -220,38 +199,37 @@ impl ImagApplication for ImagDiagnostics { let mut out = rt.stdout(); - do_write!(out, "imag version {}", { env!("CARGO_PKG_VERSION") }); - do_write!(out, ""); - do_write!(out, "{} entries", n); + write!(out, "imag version {}", { env!("CARGO_PKG_VERSION") })?; + write!(out, "")?; + write!(out, "{} entries", n)?; for (k, v) in version_counts { - do_write!(out, "{} entries with store version '{}'", v, k); + write!(out, "{} entries with store version '{}'", v, k)?; } if n != 0 { - do_write!(out, "{} header sections in the average entry", sum_header_sections / n); - do_write!(out, "{} average content bytecount", sum_bytecount_content / n); - do_write!(out, "{} average overall bytecount", sum_overall_byte_size / n); + write!(out, "{} header sections in the average entry", sum_header_sections / n)?; + write!(out, "{} average content bytecount", sum_bytecount_content / n)?; + write!(out, "{} average overall bytecount", sum_overall_byte_size / n)?; if let Some((num, path)) = max_overall_byte_size { - do_write!(out, "Largest Entry ({} bytes): {}", num, path.local_display_string()); + write!(out, "Largest Entry ({} bytes): {}", num, path.local_display_string())?; } - do_write!(out, "{} average internal link count per entry", num_links / n); + write!(out, "{} average internal link count per entry", num_links / n)?; if let Some((num, path)) = max_links { - do_write!(out, "Entry with most internal links ({}): {}", - num, - path.local_display_string()); + write!(out, "Entry with most internal links ({}): {}", + num, + path.local_display_string())?; } - do_write!(out, "{} verified entries", verified_count); - do_write!(out, "{} unverified entries", unverified_count); + write!(out, "{} verified entries", verified_count)?; + write!(out, "{} unverified entries", unverified_count)?; if verbose { for unve in unverified_entries.iter() { - do_write!(out, "Unverified: {}", unve); + write!(out, "Unverified: {}", unve)?; } } } - Ok(()) } @@ -272,17 +250,12 @@ impl ImagApplication for ImagDiagnostics { } } -fn get_config(rt: &Runtime, s: &'static str) -> Option { - rt.config().and_then(|cfg| { - cfg.read(s) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - .map(|opt| match opt { - &Value::String(ref s) => s.to_owned(), - _ => { - error!("Config type wrong: 'rt.progressbar_style' should be a string"); - ::std::process::exit(1) - } - }) - }) +fn get_config(rt: &Runtime, s: &'static str) -> Result> { + let cfg = rt.config().ok_or_else(|| err_msg("No configuration"))?; + + match cfg.read(s)? { + Some(&Value::String(ref s)) => Ok(Some(s.to_owned())), + Some(_) => Err(err_msg("Config type wrong: 'rt.progressbar_style' should be a string")), + None => Ok(None), + } } From 0e20b25091a41b0092f127df15cd444fb78853ba Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 24 Oct 2019 17:39:38 +0200 Subject: [PATCH 07/22] Add extension traits for handling Result, E> conversion This extension traits help transforming Result, E> to Result or transforming an iterator over the former type to an iterator over the latter type. Should be moved to resiter of course, but we need to implement this first. Signed-off-by: Matthias Beyer --- lib/core/libimagerror/src/iter.rs | 46 +++++++++++++++++++++++++++++ lib/core/libimagerror/src/lib.rs | 1 + lib/core/libimagerror/src/result.rs | 38 ++++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 lib/core/libimagerror/src/result.rs diff --git a/lib/core/libimagerror/src/iter.rs b/lib/core/libimagerror/src/iter.rs index 5a288321..1e7dbff9 100644 --- a/lib/core/libimagerror/src/iter.rs +++ b/lib/core/libimagerror/src/iter.rs @@ -128,3 +128,49 @@ impl TraceIterator for I where I: Iterator> {} + +/// Extension trait for doing +/// +/// ```ignore +/// Iterator, E>> -> Iterator> +/// ``` +/// +pub trait IterInnerOkOrElse + where T: Sized, + E: Sized, + Self: Iterator, E>> + Sized, + F: Fn() -> E, +{ + fn map_inner_ok_or_else(self, f: F) -> IterInnerOkOrElseImpl; +} + +pub struct IterInnerOkOrElseImpl(I, F) + where I: Iterator, E>> + Sized, + T: Sized, + E: Sized, + F: Fn() -> E; + +impl IterInnerOkOrElse for I + where I: Iterator, E>> + Sized, + T: Sized, + E: Sized, + F: Fn() -> E, +{ + fn map_inner_ok_or_else(self, f: F) -> IterInnerOkOrElseImpl { + IterInnerOkOrElseImpl(self, f) + } +} + +impl Iterator for IterInnerOkOrElseImpl + where I: Iterator, E>> + Sized, + T: Sized, + E: Sized, + F: Fn() -> E, +{ + type Item = Result; + + fn next(&mut self) -> Option { + self.0.next().map(|e| e.and_then(|opt| opt.ok_or_else(|| (self.1)()))) + } +} + diff --git a/lib/core/libimagerror/src/lib.rs b/lib/core/libimagerror/src/lib.rs index 22391142..9b5e8407 100644 --- a/lib/core/libimagerror/src/lib.rs +++ b/lib/core/libimagerror/src/lib.rs @@ -44,6 +44,7 @@ pub mod errors; pub mod exit; pub mod io; pub mod iter; +pub mod result; pub mod str; pub mod trace; diff --git a/lib/core/libimagerror/src/result.rs b/lib/core/libimagerror/src/result.rs new file mode 100644 index 00000000..0dcb5333 --- /dev/null +++ b/lib/core/libimagerror/src/result.rs @@ -0,0 +1,38 @@ +// +// imag - the personal information management suite for the commandline +// Copyright (C) 2015-2019 the imag contributors +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; version +// 2.1 of the License. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +// + +/// Extension trait for doing `Result, E> -> Result` +pub trait ResultOptionExt + where T: Sized, + E: Sized, + F: FnOnce() -> E +{ + fn inner_ok_or_else(self, f: F) -> Result; +} + +impl ResultOptionExt for Result, E> + where T: Sized, + E: Sized, + F: FnOnce() -> E +{ + fn inner_ok_or_else(self, f: F) -> Result { + self.and_then(|opt| opt.ok_or_else(f)) + } +} + From fd40715b29f63626b37ea1f99dc8715da3fceeba Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 24 Oct 2019 17:53:36 +0200 Subject: [PATCH 08/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-edit/Cargo.toml | 1 + bin/core/imag-edit/src/lib.rs | 47 ++++++++++++----------------------- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/bin/core/imag-edit/Cargo.toml b/bin/core/imag-edit/Cargo.toml index 6dc9a67d..590a9cce 100644 --- a/bin/core/imag-edit/Cargo.toml +++ b/bin/core/imag-edit/Cargo.toml @@ -25,6 +25,7 @@ version = "3.0.0" toml = "0.5.1" toml-query = "0.9.2" failure = "0.1.5" +resiter = "0.3.0" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-edit/src/lib.rs b/bin/core/imag-edit/src/lib.rs index 7d515c6d..132aaea3 100644 --- a/bin/core/imag-edit/src/lib.rs +++ b/bin/core/imag-edit/src/lib.rs @@ -37,6 +37,7 @@ extern crate clap; #[macro_use] extern crate log; extern crate failure; +extern crate resiter; extern crate libimagentryedit; extern crate libimagerror; @@ -44,16 +45,16 @@ extern crate libimagrt; extern crate libimagstore; extern crate libimagutil; -use libimagerror::trace::MapErrTrace; -use libimagerror::iter::TraceIterator; use libimagentryedit::edit::Edit; use libimagentryedit::edit::EditHeader; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; -use libimagstore::storeid::StoreIdIterator; use libimagstore::iter::get::StoreIdGetIteratorExtension; +use libimagerror::iter::IterInnerOkOrElse; use failure::Fallible as Result; +use failure::err_msg; +use resiter::AndThen; use clap::App; mod ui; @@ -68,39 +69,23 @@ impl ImagApplication for ImagEdit { let edit_header = rt.cli().is_present("edit-header"); let edit_header_only = rt.cli().is_present("edit-header-only"); - let sids = rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) - .into_iter(); - - StoreIdIterator::new(Box::new(sids.into_iter().map(Ok))) + rt.ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? + .into_iter() + .map(Ok) .into_get_iter(rt.store()) - .trace_unwrap_exit() - .map(|o| o.unwrap_or_else(|| { - error!("Did not find one entry"); - ::std::process::exit(1) - })) - .for_each(|mut entry| { + .map_inner_ok_or_else(|| err_msg("Did not find one entry")) + .inspect(|e| debug!("Editing = {:?}", e)) + .and_then_ok(|mut entry| { if edit_header { - let _ = entry - .edit_header_and_content(&rt) - .map_err_trace_exit_unwrap(); + entry.edit_header_and_content(&rt) } else if edit_header_only { - let _ = entry - .edit_header(&rt) - .map_err_trace_exit_unwrap(); + entry.edit_header(&rt) } else { - let _ = entry - .edit_content(&rt) - .map_err_trace_exit_unwrap(); + entry.edit_content(&rt) } - }); - - Ok(()) + }) + .collect() } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { From 4b8ca2b110f812a96d7026d8a853a979e857a5e8 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 09/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-git/src/lib.rs | 101 +++++++++++------------------------ 1 file changed, 31 insertions(+), 70 deletions(-) diff --git a/bin/core/imag-git/src/lib.rs b/bin/core/imag-git/src/lib.rs index b5b7d5e7..ded240b0 100644 --- a/bin/core/imag-git/src/lib.rs +++ b/bin/core/imag-git/src/lib.rs @@ -38,12 +38,11 @@ extern crate clap; #[macro_use] extern crate log; extern crate toml; extern crate toml_query; -extern crate failure; +#[macro_use] extern crate failure; extern crate libimagrt; extern crate libimagerror; -use std::io::Write; use std::io::ErrorKind; use std::process::Command; @@ -51,9 +50,9 @@ use toml::Value; use toml_query::read::TomlValueReadExt; use clap::App; use failure::Fallible as Result; +use failure::ResultExt; +use failure::err_msg; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; @@ -68,29 +67,19 @@ impl ImagApplication for ImagGit { fn run(rt: Runtime) -> Result<()> { let execute_in_store = rt .config() - .unwrap_or_else(|| { - error!("No configuration. Please use git yourself, not via imag-git"); - error!("Won't continue without configuration."); - ::std::process::exit(1); - }) + .ok_or_else(|| err_msg("No configuration. Please use git yourself, not via imag-git")) + .context("Won't continue without configuration.") + ? .read("git.execute_in_store") - .unwrap_or_else(|e| { - error!("Failed to read config setting 'git.execute_in_store'"); - error!("-> {:?}", e); - ::std::process::exit(1) - }) - .unwrap_or_else(|| { - error!("Missing config setting 'git.execute_in_store'"); - ::std::process::exit(1) - }); + .context("Failed to read config setting 'git.execute_in_store'") + ? + .ok_or_else(|| err_msg("Missing config setting 'git.execute_in_store'")) + ?; let execute_in_store = match *execute_in_store { - Value::Boolean(b) => b, - _ => { - error!("Type error: 'git.execute_in_store' is not a boolean!"); - ::std::process::exit(1) - } - }; + Value::Boolean(b) => Ok(b), + _ => Err(err_msg("Type error: 'git.execute_in_store' is not a boolean!")), + }?; let execpath = if execute_in_store { rt.store().path().to_str() @@ -98,11 +87,7 @@ impl ImagApplication for ImagGit { rt.rtp().to_str() } .map(String::from) - .unwrap_or_else(|| { - error!("Cannot parse to string: {:?}", rt.store().path()); - ::std::process::exit(1) - }); - + .ok_or_else(|| format_err!("Cannot parse to string: {:?}", rt.store().path()))?; let mut command = Command::new("git"); command @@ -120,63 +105,39 @@ impl ImagApplication for ImagGit { debug!("Adding args = {:?}", args); command.args(&args); - match rt.cli().subcommand() { - (external, Some(ext_m)) => { - command.arg(external); - let args = ext_m - .values_of("") - .map(|vs| vs.map(String::from).collect()) - .unwrap_or_else(|| vec![]); + if let (external, Some(ext_m)) = rt.cli().subcommand() { + command.arg(external); + let args = ext_m + .values_of("") + .map(|vs| vs.map(String::from).collect()) + .unwrap_or_else(|| vec![]); - debug!("Adding subcommand '{}' and args = {:?}", external, args); - command.args(&args); - }, - _ => {}, + debug!("Adding subcommand '{}' and args = {:?}", external, args); + command.args(&args); } - let mut out = rt.stdout(); - debug!("Calling: {:?}", command); match command.spawn().and_then(|mut c| c.wait()) { Ok(exit_status) => { if !exit_status.success() { debug!("git exited with non-zero exit code: {:?}", exit_status); - let mut err = rt.stderr(); - writeln!(err, "git exited with non-zero exit code") - .to_exit_code() - .unwrap_or_exit(); - ::std::process::exit(exit_status.code().unwrap_or(1)); + Err(format_err!("git exited with non-zero exit code: {:?}", exit_status)) + } else { + debug!("Successful exit!"); + Ok(()) } - debug!("Successful exit!"); }, Err(e) => { debug!("Error calling git"); - match e.kind() { - ErrorKind::NotFound => { - let _ = writeln!(out, "Cannot find 'git' executable") - .to_exit_code() - .unwrap_or_exit(); - ::std::process::exit(1); - }, - ErrorKind::PermissionDenied => { - let _ = writeln!(out, "No permission to execute: 'git'") - .to_exit_code() - .unwrap_or_exit(); - ::std::process::exit(1); - }, - _ => { - let _ = writeln!(out, "Error spawning: {:?}", e) - .to_exit_code() - .unwrap_or_exit(); - ::std::process::exit(1); - } - } + Err(match e.kind() { + ErrorKind::NotFound => err_msg("Cannot find 'git' executable"), + ErrorKind::PermissionDenied => err_msg("No permission to execute: 'git'"), + _ => format_err!("Error spawning: {:?}", e), + }) } } - - Ok(()) } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { From 05685a6f2df5d28677185fbf04dacd8be2f05168 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 10/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-gps/src/lib.rs | 155 ++++++++++++++--------------------- 1 file changed, 61 insertions(+), 94 deletions(-) diff --git a/bin/core/imag-gps/src/lib.rs b/bin/core/imag-gps/src/lib.rs index 6b5ae542..20c3b495 100644 --- a/bin/core/imag-gps/src/lib.rs +++ b/bin/core/imag-gps/src/lib.rs @@ -45,12 +45,11 @@ extern crate libimagerror; extern crate libimagstore; use std::io::Write; -use std::process::exit; use std::str::FromStr; - -use failure::err_msg; +use failure::Error; use failure::Fallible as Result; +use failure::err_msg; use clap::App; use libimagstore::storeid::StoreId; @@ -58,9 +57,6 @@ use libimagentrygps::types::*; use libimagentrygps::entry::*; use libimagrt::application::ImagApplication; use libimagrt::runtime::Runtime; -use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; mod ui; @@ -71,22 +67,22 @@ mod ui; pub enum ImagGps {} impl ImagApplication for ImagGps { fn run(rt: Runtime) -> Result<()> { - if let Some(name) = rt.cli().subcommand_name() { - match name { - "add" => add(&rt), - "remove" => remove(&rt), - "get" => get(&rt), - other => { - debug!("Unknown command"); - let _ = rt.handle_unknown_subcommand("imag-gps", other, rt.cli()) - .map_err_trace_exit_unwrap() - .code() - .map(::std::process::exit); + match rt.cli().subcommand_name().ok_or_else(|| err_msg("No subcommand called"))? { + "add" => add(&rt), + "remove" => remove(&rt), + "get" => get(&rt), + other => { + debug!("Unknown command"); + if rt.handle_unknown_subcommand("imag-gps", other, rt.cli()) + .map_err(Error::from)? + .success() + { + Ok(()) + } else { + Err(format_err!("Subcommand failed")) } } } - - Ok(()) } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { @@ -106,46 +102,33 @@ impl ImagApplication for ImagGps { } } -fn rt_get_ids(rt: &Runtime) -> Vec { +fn rt_get_ids(rt: &Runtime) -> Result> { rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + .ids::()? + .ok_or_else(|| err_msg("No ids supplied")) } -fn add(rt: &Runtime) { +fn add(rt: &Runtime) -> Result<()> { let c = { - let parse = |value: &str| -> (i64, i64, i64) { + let parse = |value: &str| -> Result<(i64, i64, i64)> { debug!("Parsing '{}' into degree, minute and second", value); let ary = value.split('.') .map(|v| {debug!("Parsing = {}", v); v}) .map(FromStr::from_str) - .map(|elem| { - elem.or_else(|_| Err(err_msg("Error while converting number"))) - .map_err_trace_exit_unwrap() - }) - .collect::>(); + .map(|elem| elem.or_else(|_| Err(err_msg("Error while converting number")))) + .collect::>>()?; - let degree = ary.get(0).unwrap_or_else(|| { - error!("Degree missing. This value is required."); - exit(1) - }); - let minute = ary.get(1).unwrap_or_else(|| { - error!("Degree missing. This value is required."); - exit(1) - }); + let degree = ary.get(0).ok_or_else(|| err_msg("Degree missing. This value is required."))?; + let minute = ary.get(1).ok_or_else(|| err_msg("Degree missing. This value is required."))?; let second = ary.get(2).unwrap_or(&0); - (*degree, *minute, *second) + Ok((*degree, *minute, *second)) }; let scmd = rt.cli().subcommand_matches("add").unwrap(); // safed by main() - let long = parse(scmd.value_of("longitude").unwrap()); // unwrap safed by clap - let lati = parse(scmd.value_of("latitude").unwrap()); // unwrap safed by clap + let long = parse(scmd.value_of("longitude").unwrap())?; // unwrap safed by clap + let lati = parse(scmd.value_of("latitude").unwrap())?; // unwrap safed by clap let long = GPSValue::new(long.0, long.1, long.2); let lati = GPSValue::new(lati.0, lati.1, lati.2); @@ -153,82 +136,66 @@ fn add(rt: &Runtime) { Coordinates::new(long, lati) }; - rt_get_ids(&rt) + rt_get_ids(&rt)? .into_iter() - .for_each(|id| { + .map(|id| { rt.store() - .get(id.clone()) - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { // if we have Ok(None) - error!("No such entry: {}", id); - exit(1) - }) - .set_coordinates(c.clone()) - .map_err_trace_exit_unwrap(); + .get(id.clone())? + .ok_or_else(|| format_err!("No such entry: {}", id))? + .set_coordinates(c.clone())?; - rt.report_touched(&id).unwrap_or_exit(); - }); + rt.report_touched(&id).map_err(Error::from) + }) + .collect() } -fn remove(rt: &Runtime) { +fn remove(rt: &Runtime) -> Result<()> { let print_removed = rt .cli() .subcommand_matches("remove") .unwrap() .is_present("print-removed"); // safed by main() - rt_get_ids(&rt) + rt_get_ids(&rt)? .into_iter() - .for_each(|id| { - let removed_value = rt + .map(|id| { + let removed_value : Coordinates = rt .store() - .get(id.clone()) - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { // if we have Ok(None) - error!("No such entry: {}", id); - exit(1) - }) - .remove_coordinates() - .map_err_trace_exit_unwrap() // The delete action failed - .unwrap_or_else(|| { // if we have Ok(None) - error!("Entry had no coordinates: {}", id); - exit(1) - }) - .map_err_trace_exit_unwrap(); // The parsing of the deleted values failed + .get(id.clone())? + .ok_or_else(|| format_err!("No such entry: {}", id))? + .remove_coordinates()? + .ok_or_else(|| format_err!("Entry had no coordinates: {}", id))??; if print_removed { - writeln!(rt.stdout(), "{}", removed_value).to_exit_code().unwrap_or_exit(); + writeln!(rt.stdout(), "{}", removed_value)?; } - rt.report_touched(&id).unwrap_or_exit(); - }); + rt.report_touched(&id).map_err(Error::from) + }) + .collect() } -fn get(rt: &Runtime) { +fn get(rt: &Runtime) -> Result<()> { let mut stdout = rt.stdout(); - rt_get_ids(&rt) + rt_get_ids(&rt)? .into_iter() - .for_each(|id| { + .map(|id| { let value = rt .store() - .get(id.clone()) - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { // if we have Ok(None) - error!("No such entry: {}", id); - exit(1) - }) - .get_coordinates() - .map_err_trace_exit_unwrap() // The get action failed - .unwrap_or_else(|| { // if we have Ok(None) - error!("Entry has no coordinates: {}", id); - exit(1) - }); + .get(id.clone())? + .ok_or_else(|| { // if we have Ok(None) + format_err!("No such entry: {}", id) + })? + .get_coordinates()? + .ok_or_else(|| { // if we have Ok(None) + format_err!("Entry has no coordinates: {}", id) + })?; - writeln!(stdout, "{}", value).to_exit_code().unwrap_or_exit(); + writeln!(stdout, "{}", value)?; - rt.report_touched(&id).unwrap_or_exit(); + rt.report_touched(&id).map_err(Error::from) }) - + .collect() } From df2c5faf7303cf3b4a1d75a5a0315854438718cd Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 11/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-grep/Cargo.toml | 1 + bin/core/imag-grep/src/lib.rs | 58 ++++++++++++++++++----------------- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/bin/core/imag-grep/Cargo.toml b/bin/core/imag-grep/Cargo.toml index a923a902..d87ffde3 100644 --- a/bin/core/imag-grep/Cargo.toml +++ b/bin/core/imag-grep/Cargo.toml @@ -23,6 +23,7 @@ maintenance = { status = "actively-developed" } log = "0.4.6" regex = "1.1.7" failure = "0.1.5" +resiter = "0.3.0" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-grep/src/lib.rs b/bin/core/imag-grep/src/lib.rs index f7a4a2d1..aadbcda5 100644 --- a/bin/core/imag-grep/src/lib.rs +++ b/bin/core/imag-grep/src/lib.rs @@ -35,9 +35,10 @@ )] #[macro_use] extern crate log; +#[macro_use] extern crate failure; extern crate clap; extern crate regex; -extern crate failure; +extern crate resiter; extern crate libimagstore; extern crate libimagrt; @@ -46,15 +47,17 @@ extern crate libimagerror; use std::io::Write; use regex::Regex; -use failure::Fallible as Result; use clap::App; +use failure::Error; +use failure::Fallible as Result; +use failure::err_msg; +use resiter::AndThen; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; use libimagstore::store::Entry; -use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; +use libimagerror::iter::IterInnerOkOrElse; + mod ui; @@ -82,34 +85,32 @@ impl ImagApplication for ImagGrep { .value_of("pattern") .map(Regex::new) .unwrap() // ensured by clap - .unwrap_or_else(|e| { - error!("Regex building error: {:?}", e); - ::std::process::exit(1) - }); + .map_err(|e| format_err!("Regex building error: {:?}", e))?; let overall_count = rt .store() - .entries() - .map_err_trace_exit_unwrap() + .entries()? .into_get_iter() - .filter_map(|res| res.map_err_trace_exit_unwrap()) - .filter_map(|entry| if pattern.is_match(entry.get_content()) { - show(&rt, &entry, &pattern, &opts, &mut count); - Some(()) - } else { - None + .map_inner_ok_or_else(|| err_msg("Entry from entries missing")) + .and_then_ok(|entry| { + if pattern.is_match(entry.get_content()) { + debug!("Matched: {}", entry.get_location()); + show(&rt, &entry, &pattern, &opts, &mut count) + } else { + debug!("Not matched: {}", entry.get_location()); + Ok(()) + } }) - .count(); + .collect::>>()? + .len(); if opts.count { - writeln!(rt.stdout(), "{}", count).to_exit_code().unwrap_or_exit(); + writeln!(rt.stdout(), "{}", count)?; } else if !opts.files_with_matches { writeln!(rt.stdout(), "Processed {} files, {} matches, {} nonmatches", overall_count, count, - overall_count - count) - .to_exit_code() - .unwrap_or_exit(); + overall_count - count)?; } Ok(()) @@ -130,27 +131,28 @@ impl ImagApplication for ImagGrep { fn version() -> &'static str { env!("CARGO_PKG_VERSION") } + } -fn show(rt: &Runtime, e: &Entry, re: &Regex, opts: &Options, count: &mut usize) { +fn show(rt: &Runtime, e: &Entry, re: &Regex, opts: &Options, count: &mut usize) -> Result<()> { if opts.files_with_matches { - writeln!(rt.stdout(), "{}", e.get_location()).to_exit_code().unwrap_or_exit(); + writeln!(rt.stdout(), "{}", e.get_location())?; } else if opts.count { *count += 1; } else { - writeln!(rt.stdout(), "{}:", e.get_location()).to_exit_code().unwrap_or_exit(); + writeln!(rt.stdout(), "{}:", e.get_location())?; for capture in re.captures_iter(e.get_content()) { for mtch in capture.iter() { if let Some(m) = mtch { - writeln!(rt.stdout(), " '{}'", m.as_str()).to_exit_code().unwrap_or_exit(); + writeln!(rt.stdout(), " '{}'", m.as_str())?; } } } - writeln!(rt.stdout()).to_exit_code().unwrap_or_exit(); + writeln!(rt.stdout())?; *count += 1; } - rt.report_touched(e.get_location()).unwrap_or_exit(); + rt.report_touched(e.get_location()).map_err(Error::from) } From f881f44d061a6628f3cbc90ad6cd1b80bf738d27 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 12/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-header/Cargo.toml | 1 + bin/core/imag-header/src/lib.rs | 216 +++++++++++++++----------------- 2 files changed, 101 insertions(+), 116 deletions(-) diff --git a/bin/core/imag-header/Cargo.toml b/bin/core/imag-header/Cargo.toml index 7caf2292..c619df43 100644 --- a/bin/core/imag-header/Cargo.toml +++ b/bin/core/imag-header/Cargo.toml @@ -26,6 +26,7 @@ toml = "0.5.1" toml-query = "0.9.2" filters = "0.3.0" failure = "0.1.5" +resiter = "0.3.0" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-header/src/lib.rs b/bin/core/imag-header/src/lib.rs index a8918fe8..84ca045e 100644 --- a/bin/core/imag-header/src/lib.rs +++ b/bin/core/imag-header/src/lib.rs @@ -34,10 +34,11 @@ extern crate clap; #[macro_use] extern crate log; +#[macro_use] extern crate failure; extern crate toml; extern crate toml_query; extern crate filters; -extern crate failure; +extern crate resiter; extern crate libimagentryedit; extern crate libimagerror; @@ -52,18 +53,17 @@ use std::string::ToString; use clap::{App, ArgMatches}; use filters::filter::Filter; use toml::Value; -use failure::{Fallible as Result, Error}; +use failure::Error; +use failure::Fallible as Result; +use failure::err_msg; +use resiter::FilterMap; +use resiter::AndThen; -use libimagerror::exit::ExitCode; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; -use libimagerror::iter::TraceIterator; -use libimagerror::trace::MapErrTrace; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; use libimagstore::iter::get::StoreIdGetIteratorExtension; use libimagstore::store::FileLockEntry; -use libimagstore::storeid::StoreIdIterator; +use libimagerror::iter::IterInnerOkOrElse; use toml_query::read::TomlValueReadExt; use toml_query::read::TomlValueReadTypeExt; @@ -85,22 +85,16 @@ impl ImagApplication for ImagHeader { trace!("list_output_with_ids = {:?}", list_output_with_ids ); trace!("list_output_with_ids_fmt = {:?}", list_output_with_ids_fmt); - let sids = rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) - .into_iter(); - - let iter = StoreIdIterator::new(Box::new(sids.map(Ok))) + let iter = rt + .ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? + .into_iter() + .map(Ok) .into_get_iter(rt.store()) - .trace_unwrap_exit() - .filter_map(|x| x); + .map_inner_ok_or_else(|| err_msg("Did not find one entry")); match rt.cli().subcommand() { - ("read", Some(mtch)) => ::std::process::exit(read(&rt, mtch, iter)), + ("read", Some(mtch)) => read(&rt, mtch, iter), ("has", Some(mtch)) => has(&rt, mtch, iter), ("hasnt", Some(mtch)) => hasnt(&rt, mtch, iter), ("int", Some(mtch)) => int(&rt, mtch, iter), @@ -109,16 +103,16 @@ impl ImagApplication for ImagHeader { ("bool", Some(mtch)) => boolean(&rt, mtch, iter), (other, _mtchs) => { debug!("Unknown command"); - ::std::process::exit({ - rt.handle_unknown_subcommand("imag-header", other, rt.cli()) - .map_err_trace_exit_unwrap() - .code() - .unwrap_or(1) - }); + if rt.handle_unknown_subcommand("imag-header", other, rt.cli()) + .map_err(Error::from)? + .success() + { + Ok(()) + } else { + Err(format_err!("Subcommand failed")) + } }, - }; - - Ok(()) + } } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { @@ -138,21 +132,20 @@ impl ImagApplication for ImagHeader { } } -fn read<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) -> i32 - where I: Iterator> +fn read<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) -> Result<()> + where I: Iterator>> { debug!("Processing headers: reading value"); let header_path = get_header_path(mtch, "header-value-path"); let mut output = rt.stdout(); trace!("Got output: {:?}", output); - iter.fold(0, |accu, entry| { + iter.and_then_ok(|entry| { trace!("Processing headers: working on {:?}", entry.get_location()); entry.get_header() - .read(header_path) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - .map(|value| { + .read(header_path)? + .ok_or_else(|| format_err!("Value not present for entry {} at {}", entry.get_location(), header_path)) + .and_then(|value| { trace!("Processing headers: Got value {:?}", value); let string_representation = match value { @@ -164,65 +157,56 @@ fn read<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) -> i32 }; if let Some(repr) = string_representation { - writeln!(output, "{}", repr) + writeln!(output, "{}", repr)?; } else { - writeln!(output, "{}", value) + writeln!(output, "{}", value)?; } - .to_exit_code() - .map(|_| accu) - .unwrap_or_else(ExitCode::code) - }) - .unwrap_or_else(|| { - // if value not present and configured - error!("Value not present for entry {} at {}", entry.get_location(), header_path); - 1 + Ok(()) }) }) + .collect::>() } -fn has<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) - where I: Iterator> +fn has<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) -> Result<()> + where I: Iterator>> { debug!("Processing headers: has value"); let header_path = get_header_path(mtch, "header-value-path"); let mut output = rt.stdout(); - iter.for_each(|entry| { + iter.and_then_ok(|entry| { trace!("Processing headers: working on {:?}", entry.get_location()); - if entry.get_header() - .read(header_path) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - .is_some() - { - rt.report_touched(entry.get_location()).unwrap_or_exit(); - if !rt.output_is_pipe() { - writeln!(output, "{}", entry.get_location()).to_exit_code().unwrap_or_exit(); - } + if let Some(_) = entry.get_header().read(header_path)? { + if !rt.output_is_pipe() { + writeln!(output, "{}", entry.get_location())?; } + rt.report_touched(entry.get_location()).map_err(Error::from) + } else { + Ok(()) + } }) + .collect::>() } -fn hasnt<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) - where I: Iterator> +fn hasnt<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) -> Result<()> + where I: Iterator>> { debug!("Processing headers: hasnt value"); let header_path = get_header_path(mtch, "header-value-path"); let mut output = rt.stdout(); - iter.for_each(|entry| { + iter.and_then_ok(|entry| { trace!("Processing headers: working on {:?}", entry.get_location()); - if entry.get_header() - .read(header_path) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - .is_none() { - rt.report_touched(entry.get_location()).unwrap_or_exit(); - if !rt.output_is_pipe() { - writeln!(output, "{}", entry.get_location()).to_exit_code().unwrap_or_exit(); - } + if let Some(_) = entry.get_header().read(header_path)? { + Ok(()) + } else { + if !rt.output_is_pipe() { + writeln!(output, "{}", entry.get_location())?; } + rt.report_touched(entry.get_location()).map_err(Error::from) + } }) + .collect() } macro_rules! implement_compare { @@ -238,8 +222,8 @@ macro_rules! implement_compare { }} } -fn int<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) - where I: Iterator> +fn int<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) -> Result<()> + where I: Iterator>> { debug!("Processing headers: int value"); let header_path = get_header_path(mtch, "header-value-path"); @@ -264,20 +248,20 @@ fn int<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) implement_compare!(mtch, "header-int-gte", i64, |cmp| *i >= cmp) }); - iter.filter(|entry| if let Some(hdr) = entry.get_header() - .read_int(header_path) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - { - filter.filter(&hdr) + iter.and_then_ok(|entry| { + if let Some(hdr) = entry.get_header().read_int(header_path)? { + Ok((filter.filter(&hdr), entry)) } else { - false - }) - .for_each(|entry| rt.report_touched(entry.get_location()).unwrap_or_exit()) + Ok((false, entry)) + } + }) + .filter_map_ok(|(b, e)| if b { Some(e) } else { None }) + .and_then_ok(|entry| rt.report_touched(entry.get_location()).map_err(Error::from)) + .collect() } -fn float<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) - where I: Iterator> +fn float<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) -> Result<()> + where I: Iterator>> { debug!("Processing headers: float value"); let header_path = get_header_path(mtch, "header-value-path"); @@ -302,20 +286,20 @@ fn float<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) implement_compare!(mtch, "header-float-gte", f64, |cmp| *i >= cmp) }); - iter.filter(|entry| if let Some(hdr) = entry.get_header() - .read_float(header_path) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - { - filter.filter(&hdr) + iter.and_then_ok(|entry| { + if let Some(hdr) = entry.get_header().read_float(header_path)? { + Ok((filter.filter(&hdr), entry)) } else { - false - }) - .for_each(|entry| rt.report_touched(entry.get_location()).unwrap_or_exit()) + Ok((false, entry)) + } + }) + .filter_map_ok(|(b, e)| if b { Some(e) } else { None }) + .and_then_ok(|entry| rt.report_touched(entry.get_location()).map_err(Error::from)) + .collect() } -fn string<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) - where I: Iterator> +fn string<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) -> Result<()> + where I: Iterator>> { debug!("Processing headers: string value"); let header_path = get_header_path(mtch, "header-value-path"); @@ -328,20 +312,20 @@ fn string<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) implement_compare!(mtch, "header-string-neq", String, |cmp| *i != cmp) }); - iter.filter(|entry| if let Some(hdr) = entry.get_header() - .read_string(header_path) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - { - filter.filter(&hdr) + iter.and_then_ok(|entry| { + if let Some(hdr) = entry.get_header().read_string(header_path)? { + Ok((filter.filter(&hdr), entry)) } else { - false - }) - .for_each(|entry| rt.report_touched(entry.get_location()).unwrap_or_exit()) + Ok((false, entry)) + } + }) + .filter_map_ok(|(b, e)| if b { Some(e) } else { None }) + .and_then_ok(|entry| rt.report_touched(entry.get_location()).map_err(Error::from)) + .collect() } -fn boolean<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) - where I: Iterator> +fn boolean<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) -> Result<()> + where I: Iterator>> { debug!("Processing headers: bool value"); let header_path = get_header_path(mtch, "header-value-path"); @@ -350,16 +334,16 @@ fn boolean<'a, 'e, I>(rt: &Runtime, mtch: &ArgMatches<'a>, iter: I) .and(|i: &bool| -> bool { *i }) .and(|i: &bool| -> bool { *i }); - iter.filter(|entry| if let Some(hdr) = entry.get_header() - .read_bool(header_path) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - { - filter.filter(&hdr) + iter.and_then_ok(|entry| { + if let Some(hdr) = entry.get_header().read_bool(header_path)? { + Ok((filter.filter(&hdr), entry)) } else { - false - }) - .for_each(|entry| rt.report_touched(entry.get_location()).unwrap_or_exit()) + Ok((false, entry)) + } + }) + .filter_map_ok(|(b, e)| if b { Some(e) } else { None }) + .and_then_ok(|entry| rt.report_touched(entry.get_location()).map_err(Error::from)) + .collect() } From 0e7422509408b771ac8ad315fd7bd5c6807e1c1e Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 13/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-id-in-collection/src/main.rs | 27 ++++++++++------------ 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/bin/core/imag-id-in-collection/src/main.rs b/bin/core/imag-id-in-collection/src/main.rs index 8357828a..1821bbf8 100644 --- a/bin/core/imag-id-in-collection/src/main.rs +++ b/bin/core/imag-id-in-collection/src/main.rs @@ -51,12 +51,12 @@ extern crate libimagstore; use std::io::Write; use filters::filter::Filter; +use failure::Fallible as Result; +use failure::Error; +use failure::err_msg; use libimagstore::storeid::StoreId; use libimagrt::setup::generate_runtime_setup; -use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; mod ui; @@ -83,7 +83,7 @@ impl<'a, A> Filter for IsInCollectionsFilter<'a, A> } -fn main() { +fn main() -> Result<()> { let version = make_imag_version!(); let rt = generate_runtime_setup("imag-id-in-collection", &version, @@ -99,22 +99,19 @@ fn main() { let mut stdout = rt.stdout(); trace!("Got output: {:?}", stdout); - - rt.ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + rt.ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? .iter() .filter(|id| collection_filter.filter(id)) - .for_each(|id| { - rt.report_touched(&id).unwrap_or_exit(); + .map(|id| { if !rt.output_is_pipe() { - let id = id.to_str().map_err_trace_exit_unwrap(); + let id = id.to_str()?; trace!("Writing to {:?}", stdout); - writeln!(stdout, "{}", id).to_exit_code().unwrap_or_exit(); + writeln!(stdout, "{}", id)?; } + + rt.report_touched(&id).map_err(Error::from) }) + .collect() } From 9b8faec86e4ff93b382e855d809b2d3065c5592e Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 14/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-view/Cargo.toml | 1 + bin/core/imag-view/src/lib.rs | 161 ++++++++++++---------------------- 2 files changed, 57 insertions(+), 105 deletions(-) diff --git a/bin/core/imag-view/Cargo.toml b/bin/core/imag-view/Cargo.toml index 0525afec..c5b4e3fe 100644 --- a/bin/core/imag-view/Cargo.toml +++ b/bin/core/imag-view/Cargo.toml @@ -26,6 +26,7 @@ toml-query = "0.9.2" handlebars = "2" tempfile = "3.0.9" failure = "0.1.5" +resiter = "0.3.0" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-view/src/lib.rs b/bin/core/imag-view/src/lib.rs index 7e9127cc..6e721531 100644 --- a/bin/core/imag-view/src/lib.rs +++ b/bin/core/imag-view/src/lib.rs @@ -40,7 +40,8 @@ extern crate handlebars; extern crate tempfile; extern crate toml; extern crate toml_query; -extern crate failure; +#[macro_use] extern crate failure; +extern crate resiter; extern crate libimagentryview; extern crate libimagerror; @@ -52,26 +53,23 @@ use std::str::FromStr; use std::collections::BTreeMap; use std::io::Write; use std::process::Command; -use std::process::exit; use handlebars::Handlebars; use toml_query::read::TomlValueReadTypeExt; use failure::Error; use failure::err_msg; use failure::Fallible as Result; +use resiter::AndThen; use clap::App; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; -use libimagerror::trace::MapErrTrace; -use libimagerror::iter::TraceIterator; -use libimagerror::io::ToExitCode; -use libimagerror::exit::ExitUnwrap; use libimagentryview::builtin::stdout::StdoutViewer; use libimagentryview::builtin::md::MarkdownViewer; use libimagentryview::viewer::Viewer; use libimagstore::iter::get::StoreIdGetIteratorExtension; use libimagstore::store::FileLockEntry; +use libimagerror::iter::IterInnerOkOrElse; mod ui; @@ -85,61 +83,42 @@ impl ImagApplication for ImagView { let view_header = rt.cli().is_present("view-header"); let hide_content = rt.cli().is_present("not-view-content"); let entries = rt - .ids::<::ui::PathProvider>() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + .ids::<::ui::PathProvider>()? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter() .map(Ok) .into_get_iter(rt.store()) - .trace_unwrap_exit() - .map(|e| { - e.ok_or_else(|| err_msg("Entry not found")) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - }); + .map_inner_ok_or_else(|| err_msg("Entry not found, please report this as a bug")); if rt.cli().is_present("in") { let files = entries - .map(|entry| { - let tmpfile = create_tempfile_for(&entry, view_header, hide_content); - rt.report_touched(entry.get_location()).unwrap_or_exit(); - tmpfile + .and_then_ok(|entry| { + let tmpfile = create_tempfile_for(&entry, view_header, hide_content)?; + rt.report_touched(entry.get_location())?; + Ok(tmpfile) }) - .collect::>(); + .collect::>>()?; let mut command = { let viewer = rt .cli() .value_of("in") - .ok_or_else(|| Error::from(err_msg("No viewer given"))) - .map_err_trace_exit_unwrap(); + .ok_or_else(|| err_msg("No viewer given"))?; let config = rt .config() - .ok_or_else(|| Error::from(err_msg("No configuration, cannot continue"))) - .map_err_trace_exit_unwrap(); + .ok_or_else(|| err_msg("No configuration, cannot continue"))?; let query = format!("view.viewers.{}", viewer); let viewer_template = config - .read_string(&query) - .map_err(Error::from) - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("Cannot find '{}' in config", query); - exit(1) - }); + .read_string(&query)? + .ok_or_else(|| format_err!("Cannot find '{}' in config", query))?; let mut handlebars = Handlebars::new(); handlebars.register_escape_fn(::handlebars::no_escape); - let _ = handlebars - .register_template_string("template", viewer_template) - .map_err(Error::from) - .map_err_trace_exit_unwrap(); + handlebars.register_template_string("template", viewer_template)?; let mut data = BTreeMap::new(); @@ -151,15 +130,9 @@ impl ImagApplication for ImagView { data.insert("entries", file_paths); - let call = handlebars - .render("template", &data) - .map_err(Error::from) - .map_err_trace_exit_unwrap(); + let call = handlebars .render("template", &data)?; let mut elems = call.split_whitespace(); - let command_string = elems - .next() - .ok_or_else(|| Error::from(err_msg("No command"))) - .map_err_trace_exit_unwrap(); + let command_string = elems.next().ok_or_else(|| err_msg("No command"))?; let mut cmd = Command::new(command_string); for arg in elems { @@ -172,15 +145,14 @@ impl ImagApplication for ImagView { debug!("Calling: {:?}", command); if !command - .status() - .map_err(Error::from) - .map_err_trace_exit_unwrap() + .status()? .success() { - exit(1) + return Err(err_msg("Failed to execute command")) } drop(files); + Ok(()) } else { let out = rt.stdout(); let mut outlock = out.lock(); @@ -202,32 +174,30 @@ impl ImagApplication for ImagView { let viewer = MarkdownViewer::new(&rt); let seperator = basesep.map(|s| build_seperator(s, sep_width)); - entries - .enumerate() - .for_each(|(n, entry)| { - if n != 0 { - seperator - .as_ref() - .map(|s| writeln!(outlock, "{}", s).to_exit_code().unwrap_or_exit()); - } + let mut i = 0; // poor mans enumerate() - if let Err(e) = viewer.view_entry(&entry, &mut outlock) { - handle_error(e); + entries.and_then_ok(|entry| { + if i != 0 { + if let Some(s) = seperator.as_ref() { + writeln!(outlock, "{}", s)?; } + } - rt.report_touched(entry.get_location()).unwrap_or_exit(); - }); + viewer.view_entry(&entry, &mut outlock)?; + + i += 1; + rt.report_touched(entry.get_location()).map_err(Error::from) + }) + .collect() } else { let mut viewer = StdoutViewer::new(view_header, !hide_content); if rt.cli().occurrences_of("autowrap") != 0 { let width = rt.cli().value_of("autowrap").unwrap(); // ensured by clap - let width = usize::from_str(width).unwrap_or_else(|e| { - error!("Failed to parse argument to number: autowrap = {:?}", - rt.cli().value_of("autowrap").map(String::from)); - error!("-> {:?}", e); - ::std::process::exit(1) - }); + let width = usize::from_str(width).map_err(|_| { + format_err!("Failed to parse argument to number: autowrap = {:?}", + rt.cli().value_of("autowrap").map(String::from)) + })?; // Copying this value over, so that the seperator has the right len as well sep_width = width; @@ -236,25 +206,22 @@ impl ImagApplication for ImagView { } let seperator = basesep.map(|s| build_seperator(s, sep_width)); - entries - .enumerate() - .for_each(|(n, entry)| { - if n != 0 { - seperator - .as_ref() - .map(|s| writeln!(outlock, "{}", s).to_exit_code().unwrap_or_exit()); + let mut i = 0; // poor mans enumerate() + entries.and_then_ok(|entry| { + if i != 0 { + if let Some(s) = seperator.as_ref() { + writeln!(outlock, "{}", s)?; } + } - if let Err(e) = viewer.view_entry(&entry, &mut outlock) { - handle_error(e); - } + viewer.view_entry(&entry, &mut outlock)?; - rt.report_touched(entry.get_location()).unwrap_or_exit(); - }); + i += 1; + rt.report_touched(entry.get_location()).map_err(Error::from) + }) + .collect() } } - - Ok(()) } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { @@ -275,41 +242,25 @@ impl ImagApplication for ImagView { } fn create_tempfile_for<'a>(entry: &FileLockEntry<'a>, view_header: bool, hide_content: bool) - -> (tempfile::NamedTempFile, String) + -> Result<(tempfile::NamedTempFile, String)> { - let mut tmpfile = tempfile::NamedTempFile::new() - .map_err(Error::from) - .map_err_trace_exit_unwrap(); + let mut tmpfile = tempfile::NamedTempFile::new()?; if view_header { - let hdr = toml::ser::to_string_pretty(entry.get_header()) - .map_err(Error::from) - .map_err_trace_exit_unwrap(); - let _ = tmpfile.write(format!("---\n{}---\n", hdr).as_bytes()) - .map_err(Error::from) - .map_err_trace_exit_unwrap(); + let hdr = toml::ser::to_string_pretty(entry.get_header())?; + let _ = tmpfile.write(format!("---\n{}---\n", hdr).as_bytes())?; } if !hide_content { - let _ = tmpfile.write(entry.get_content().as_bytes()) - .map_err(Error::from) - .map_err_trace_exit_unwrap(); + let _ = tmpfile.write(entry.get_content().as_bytes())?; } let file_path = tmpfile .path() .to_str() .map(String::from) - .ok_or_else(|| Error::from(err_msg("Cannot build path"))) - .map_err_trace_exit_unwrap(); + .ok_or_else(|| Error::from(err_msg("Cannot build path")))?; - (tmpfile, file_path) + Ok((tmpfile, file_path)) } -fn handle_error(e: ::libimagentryview::error::Error) { - use libimagentryview::error::Error; - match e { - Error::Io(e) => Err(e).to_exit_code().unwrap_or_exit(), - Error::Other(e) => Err(e).map_err_trace_exit_unwrap() - } -} From eb8912ffe633eca254def11095f66ab752ca238c Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 15/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-ids/Cargo.toml | 1 + bin/core/imag-ids/src/lib.rs | 76 +++++++++++++++++------------------- 2 files changed, 36 insertions(+), 41 deletions(-) diff --git a/bin/core/imag-ids/Cargo.toml b/bin/core/imag-ids/Cargo.toml index 8f6afe96..4dcd489f 100644 --- a/bin/core/imag-ids/Cargo.toml +++ b/bin/core/imag-ids/Cargo.toml @@ -24,6 +24,7 @@ log = "0.4.6" toml = "0.5.1" toml-query = "0.9.2" failure = "0.1.5" +resiter = "0.3.0" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-ids/src/lib.rs b/bin/core/imag-ids/src/lib.rs index bb0e08a8..43849462 100644 --- a/bin/core/imag-ids/src/lib.rs +++ b/bin/core/imag-ids/src/lib.rs @@ -39,6 +39,7 @@ extern crate clap; extern crate toml; extern crate toml_query; #[macro_use] extern crate failure; +extern crate resiter; #[cfg(test)] extern crate env_logger; @@ -48,18 +49,17 @@ extern crate libimagstore; extern crate libimagrt; use std::io::Write; -use std::result::Result as RResult; use failure::Fallible as Result; +use failure::err_msg; +use failure::Error; +use resiter::Map; +use resiter::AndThen; use clap::App; use libimagstore::storeid::StoreId; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; -use libimagerror::trace::MapErrTrace; -use libimagerror::iter::TraceIterator; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; mod ui; @@ -72,48 +72,42 @@ impl ImagApplication for ImagIds { fn run(rt: Runtime) -> Result<()> { let print_storepath = rt.cli().is_present("print-storepath"); - let iterator = if rt.ids_from_stdin() { - debug!("Fetching IDs from stdin..."); - let ids = rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }); - Box::new(ids.into_iter().map(Ok)) - as Box>> - } else { - Box::new(rt.store().entries().map_err_trace_exit_unwrap()) - as Box>> - } - .trace_unwrap_exit() - .map(|id| if print_storepath { - (Some(rt.store().path()), id) - } else { - (None, id) - }); - let mut stdout = rt.stdout(); trace!("Got output: {:?}", stdout); - iterator.for_each(|(storepath, id)| { - rt.report_touched(&id).unwrap_or_exit(); - if !rt.output_is_pipe() { - let id = id.to_str().map_err_trace_exit_unwrap(); - trace!("Writing to {:?}", stdout); + let mut process = |iter: &mut dyn Iterator>| -> Result<()> { + iter.map_ok(|id| if print_storepath { + (Some(rt.store().path()), id) + } else { + (None, id) + }).and_then_ok(|(storepath, id)| { + if !rt.output_is_pipe() { + let id = id.to_str()?; + trace!("Writing to {:?}", stdout); - let result = if let Some(store) = storepath { - writeln!(stdout, "{}/{}", store.display(), id) - } else { - writeln!(stdout, "{}", id) - }; + if let Some(store) = storepath { + writeln!(stdout, "{}/{}", store.display(), id)?; + } else { + writeln!(stdout, "{}", id)?; + } + } - result.to_exit_code().unwrap_or_exit(); - } - }); + rt.report_touched(&id).map_err(Error::from) + }) + .collect::>() + }; - Ok(()) + if rt.ids_from_stdin() { + debug!("Fetching IDs from stdin..."); + let mut iter = rt.ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? + .into_iter() + .map(Ok); + + process(&mut iter) + } else { + process(&mut rt.store().entries()?) + } } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { From 2811ad3d4d431821aa0e7f86664f5bff1e3f6698 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 16/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-init/src/bin.rs | 3 +- bin/core/imag-init/src/lib.rs | 148 +++++++++++++++------------------- 2 files changed, 65 insertions(+), 86 deletions(-) diff --git a/bin/core/imag-init/src/bin.rs b/bin/core/imag-init/src/bin.rs index aea6ebc3..79cc3f94 100644 --- a/bin/core/imag-init/src/bin.rs +++ b/bin/core/imag-init/src/bin.rs @@ -41,6 +41,5 @@ extern crate libimaginitcmd; fn main() -> Result<()> { - libimaginitcmd::imag_init(); - Ok(()) + libimaginitcmd::imag_init() } diff --git a/bin/core/imag-init/src/lib.rs b/bin/core/imag-init/src/lib.rs index e2033e3d..a9fda27b 100644 --- a/bin/core/imag-init/src/lib.rs +++ b/bin/core/imag-init/src/lib.rs @@ -35,7 +35,8 @@ )] extern crate clap; -extern crate failure; +#[macro_use] extern crate failure; + #[cfg(test)] extern crate toml; @@ -50,12 +51,14 @@ use std::path::PathBuf; use std::path::Path; use std::process::Command; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; +use failure::Fallible as Result; +use failure::ResultExt; +use failure::Error; +use failure::err_msg; + use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; -use failure::Fallible as Result; use clap::App; const CONFIGURATION_STR : &str = include_str!("../imagrc.toml"); @@ -100,7 +103,7 @@ impl ImagApplication for ImagInit { } } -pub fn imag_init() { +pub fn imag_init() -> Result<()> { let version = make_imag_version!(); let app = ui::build_ui(Runtime::get_default_cli_builder( "imag-init", @@ -109,35 +112,27 @@ pub fn imag_init() { let matches = app.get_matches(); let mut out = ::std::io::stdout(); - let path = matches - .value_of("path") - .map(String::from) - .map(PathBuf::from) - .unwrap_or_else(|| { - ::std::env::var("HOME") - .map(PathBuf::from) - .map(|mut p| { p.push(".imag"); p }) - .map(|path| if path.exists() { - writeln!(out, "Path '{:?}' already exists!", path) - .to_exit_code() - .unwrap_or_exit(); - writeln!(out, "Cannot continue.") - .to_exit_code() - .unwrap_or_exit(); - ::std::process::exit(1) - } else { - path - }) - .expect("Failed to retrieve/build path for imag directory.") - }); + let path = if let Some(p) = matches.value_of("path") { + PathBuf::from(String::from(p)) + } else { + ::std::env::var("HOME") + .map_err(Error::from) + .map(PathBuf::from) + .map(|mut p| { p.push(".imag"); p }) + .and_then(|path| if path.exists() { + Err(format_err!("Cannot continue: Path '{}' already exists", path.display())) + } else { + Ok(path) + }) + .map_err(|_| err_msg("Failed to retrieve/build path for imag directory."))? + }; { let mut store_path = path.clone(); store_path.push("store"); println!("Creating {}", store_path.display()); - ::std::fs::create_dir_all(store_path) - .expect("Failed to create directory"); + ::std::fs::create_dir_all(store_path).context("Failed to create directory")?; } let config_path = { @@ -146,11 +141,12 @@ pub fn imag_init() { config_path }; - OpenOptions::new() + let _ = OpenOptions::new() .write(true) .create(true) .open(config_path) - .map(|mut f| { + .map_err(Error::from) + .and_then(|mut f| { let content = if matches.is_present("devel") { get_config_devel() } else { @@ -158,33 +154,34 @@ pub fn imag_init() { }; f.write_all(content.as_bytes()) - .expect("Failed to write complete config to file"); + .context("Failed to write complete config to file") + .map_err(Error::from) }) - .expect("Failed to open new configuration file"); + .context("Failed to open new configuration file")?; if find_command("git").is_some() && !matches.is_present("nogit") { // we initialize a git repository - writeln!(out, "Going to initialize a git repository in the imag directory...") - .to_exit_code() - .unwrap_or_exit(); + writeln!(out, "Going to initialize a git repository in the imag directory...")?; let gitignore_path = { let mut gitignore_path = path.clone(); gitignore_path.push(".gitignore"); - gitignore_path.to_str().map(String::from).expect("Cannot convert path to string") - }; + gitignore_path.to_str().map(String::from) + }.ok_or_else(|| err_msg("Cannot convert path to string"))?; - OpenOptions::new() + let _ = OpenOptions::new() .write(true) .create(true) .open(gitignore_path.clone()) - .map(|mut f| { + .map_err(Error::from) + .and_then(|mut f| { f.write_all(GITIGNORE_STR.as_bytes()) - .expect("Failed to write complete gitignore to file"); + .context("Failed to write complete gitignore to file") + .map_err(Error::from) }) - .expect("Failed to open new configuration file"); + .context("Failed to open new configuration file")?; - let path_str = path.to_str().map(String::from).expect("Cannot convert path to string"); + let path_str = path.to_str().map(String::from).ok_or_else(|| err_msg("Cannot convert path to string"))?; let worktree = format!("--work-tree={}", path_str); let gitdir = format!("--git-dir={}/.git", path_str); @@ -192,20 +189,16 @@ pub fn imag_init() { let output = Command::new("git") .args(&[&worktree, &gitdir, "--no-pager", "init"]) .output() - .expect("Calling 'git init' failed"); + .context("Calling 'git init' failed")?; if output.status.success() { - writeln!(out, "{}", String::from_utf8(output.stdout).expect("No UTF-8 output")) - .to_exit_code() - .unwrap_or_exit(); - writeln!(out, "'git {} {} --no-pager init' succeeded", worktree, gitdir) - .to_exit_code() - .unwrap_or_exit(); + writeln!(out, "{}", String::from_utf8(output.stdout).expect("No UTF-8 output"))?; + writeln!(out, "'git {} {} --no-pager init' succeeded", worktree, gitdir)?; } else { - writeln!(out, "{}", String::from_utf8(output.stderr).expect("No UTF-8 output")) - .to_exit_code() - .unwrap_or_exit(); - ::std::process::exit(output.status.code().unwrap_or(1)); + writeln!(out, "{}", String::from_utf8(output.stderr).expect("No UTF-8 output"))?; + if !output.status.success() { + return Err(err_msg("Failed to execute git command")); + } } } @@ -213,19 +206,16 @@ pub fn imag_init() { let output = Command::new("git") .args(&[&worktree, &gitdir, "--no-pager", "add", &gitignore_path]) .output() - .expect("Calling 'git add' failed"); + .context("Calling 'git add' failed")?; + if output.status.success() { - writeln!(out, "{}", String::from_utf8(output.stdout).expect("No UTF-8 output")) - .to_exit_code() - .unwrap_or_exit(); - writeln!(out, "'git {} {} --no-pager add {}' succeeded", worktree, gitdir, gitignore_path) - .to_exit_code() - .unwrap_or_exit(); + writeln!(out, "{}", String::from_utf8(output.stdout).expect("No UTF-8 output"))?; + writeln!(out, "'git {} {} --no-pager add {}' succeeded", worktree, gitdir, gitignore_path)?; } else { - writeln!(out, "{}", String::from_utf8(output.stderr).expect("No UTF-8 output")) - .to_exit_code() - .unwrap_or_exit(); - ::std::process::exit(output.status.code().unwrap_or(1)); + writeln!(out, "{}", String::from_utf8(output.stderr).expect("No UTF-8 output"))?; + if !output.status.success() { + return Err(err_msg("Failed to execute git command")); + } } } @@ -233,34 +223,24 @@ pub fn imag_init() { let output = Command::new("git") .args(&[&worktree, &gitdir, "--no-pager", "commit", &gitignore_path, "-m", "'Initial import'"]) .output() - .expect("Calling 'git commit' failed"); + .context("Calling 'git commit' failed")?; if output.status.success() { - writeln!(out, "{}", String::from_utf8(output.stdout).expect("No UTF-8 output")) - .to_exit_code() - .unwrap_or_exit(); - writeln!(out, "'git {} {} --no-pager commit {} -m 'Initial import'' succeeded", worktree, gitdir, gitignore_path) - .to_exit_code() - .unwrap_or_exit(); + writeln!(out, "{}", String::from_utf8(output.stdout).expect("No UTF-8 output"))?; + writeln!(out, "'git {} {} --no-pager commit {} -m 'Initial import'' succeeded", worktree, gitdir, gitignore_path)?; } else { - writeln!(out, "{}", String::from_utf8(output.stderr).expect("No UTF-8 output")) - .to_exit_code() - .unwrap_or_exit(); - ::std::process::exit(output.status.code().unwrap_or(1)); + writeln!(out, "{}", String::from_utf8(output.stderr).expect("No UTF-8 output"))?; + if !output.status.success() { + return Err(err_msg("Failed to execute git command")); + } } } - writeln!(out, "git stuff finished!") - .to_exit_code() - .unwrap_or_exit(); + writeln!(out, "git stuff finished!")?; } else { - writeln!(out, "No git repository will be initialized") - .to_exit_code() - .unwrap_or_exit(); + writeln!(out, "No git repository will be initialized")?; } - writeln!(out, "Ready. Have fun with imag!") - .to_exit_code() - .unwrap_or_exit(); + writeln!(out, "Ready. Have fun with imag!").map_err(Error::from) } fn get_config() -> String { From 0e14f953ca72a98ee7dc51fe70a0341e4b04eee6 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 17/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-link/src/lib.rs | 324 +++++++++++++--------------------- 1 file changed, 122 insertions(+), 202 deletions(-) diff --git a/bin/core/imag-link/src/lib.rs b/bin/core/imag-link/src/lib.rs index 47851127..1da4172a 100644 --- a/bin/core/imag-link/src/lib.rs +++ b/bin/core/imag-link/src/lib.rs @@ -37,7 +37,7 @@ #[macro_use] extern crate log; extern crate clap; extern crate url; -extern crate failure; +#[macro_use] extern crate failure; #[macro_use] extern crate prettytable; #[cfg(test)] extern crate toml; #[cfg(test)] extern crate toml_query; @@ -65,18 +65,14 @@ use failure::err_msg; use libimagentryurl::linker::UrlLinker; use libimagentrylink::linkable::Linkable; use libimagentrylink::storecheck::StoreLinkConsistentExt; -use libimagerror::trace::{MapErrTrace, trace_error}; -use libimagerror::exit::ExitUnwrap; -use libimagerror::io::ToExitCode; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; use libimagstore::store::FileLockEntry; use libimagstore::storeid::StoreId; -use libimagutil::warn_exit::warn_exit; -use libimagutil::warn_result::*; use url::Url; use failure::Fallible as Result; +use failure::Error; use clap::App; mod ui; @@ -89,46 +85,31 @@ pub enum ImagLink {} impl ImagApplication for ImagLink { fn run(rt: Runtime) -> Result<()> { if rt.cli().is_present("check-consistency") { - let exit_code = match rt.store().check_link_consistency() { - Ok(_) => { - info!("Store is consistent"); - 0 - } - Err(e) => { - trace_error(&e); - 1 - } - }; - ::std::process::exit(exit_code); + rt.store().check_link_consistency()?; + info!("Store is consistent"); } - let _ = rt.cli() - .subcommand_name() - .map(|name| { - match name { - "remove" => remove_linking(&rt), - "unlink" => unlink(&rt), - "list" => list_linkings(&rt), - other => { - debug!("Unknown command"); - let _ = rt.handle_unknown_subcommand("imag-link", other, rt.cli()) - .map_err_trace_exit_unwrap() - .code() - .map(::std::process::exit); - }, - } - }) - .or_else(|| { - if let (Some(from), Some(to)) = (rt.cli().value_of("from"), rt.cli().values_of("to")) { - Some(link_from_to(&rt, from, to)) - } else { - warn_exit("No commandline call", 1) - } - }) - .ok_or_else(|| err_msg("No commandline call".to_owned())) - .map_err_trace_exit_unwrap(); - - Ok(()) + if let Some(name) = rt.cli().subcommand_name() { + match name { + "remove" => remove_linking(&rt), + "unlink" => unlink(&rt), + "list" => list_linkings(&rt), + other => { + debug!("Unknown command"); + if rt.handle_unknown_subcommand("imag-link", other, rt.cli())?.success() { + Ok(()) + } else { + Err(format_err!("Subcommand failed")) + } + }, + } + } else { + if let (Some(from), Some(to)) = (rt.cli().value_of("from"), rt.cli().values_of("to")) { + link_from_to(&rt, from, to) + } else { + Err(err_msg("No commandline call")) + } + } } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { @@ -156,144 +137,103 @@ fn get_entry_by_name<'a>(rt: &'a Runtime, name: &str) -> Result(rt: &'a Runtime, from: &'a str, to: I) +fn link_from_to<'a, I>(rt: &'a Runtime, from: &'a str, to: I) -> Result<()> where I: Iterator { - let mut from_entry = match get_entry_by_name(rt, from).map_err_trace_exit_unwrap() { - Some(e) => e, - None => { - debug!("No 'from' entry"); - warn_exit("No 'from' entry", 1) - }, - }; + let mut from_entry = get_entry_by_name(rt, from)?.ok_or_else(|| err_msg("No 'from' entry"))?; for entry in to { debug!("Handling 'to' entry: {:?}", entry); - if rt.store().get(PathBuf::from(entry)).map_err_trace_exit_unwrap().is_none() { + if rt.store().get(PathBuf::from(entry))?.is_none() { debug!("Linking externally: {:?} -> {:?}", from, entry); - let url = Url::parse(entry).unwrap_or_else(|e| { - error!("Error parsing URL: {:?}", e); - ::std::process::exit(1); - }); + let url = Url::parse(entry).map_err(|e| format_err!("Error parsing URL: {:?}", e))?; let iter = from_entry - .add_url(rt.store(), url) - .map_err_trace_exit_unwrap() + .add_url(rt.store(), url)? .into_iter(); - rt.report_all_touched(iter).unwrap_or_exit(); + rt.report_all_touched(iter)?; } else { debug!("Linking internally: {:?} -> {:?}", from, entry); - let from_id = StoreId::new(PathBuf::from(from)).map_err_trace_exit_unwrap(); - let entr_id = StoreId::new(PathBuf::from(entry)).map_err_trace_exit_unwrap(); + let from_id = StoreId::new(PathBuf::from(from))?; + let entr_id = StoreId::new(PathBuf::from(entry))?; if from_id == entr_id { - error!("Cannot link entry with itself. Exiting"); - ::std::process::exit(1) + return Err(err_msg("Cannot link entry with itself. Exiting")) } - let mut to_entry = match rt.store().get(entr_id).map_err_trace_exit_unwrap() { - Some(e) => e, - None => { - warn!("No 'to' entry: {}", entry); - ::std::process::exit(1) - }, - }; - from_entry - .add_link(&mut to_entry) - .map_err_trace_exit_unwrap(); + let mut to_entry = rt + .store() + .get(entr_id)? + .ok_or_else(|| format_err!("No 'to' entry: {}", entry))?; - rt.report_touched(to_entry.get_location()).unwrap_or_exit(); + from_entry.add_link(&mut to_entry)?; + + rt.report_touched(to_entry.get_location())?; } - info!("Ok: {} -> {}", from, entry); } - rt.report_touched(from_entry.get_location()).unwrap_or_exit(); + rt.report_touched(from_entry.get_location()).map_err(Error::from) } -fn remove_linking(rt: &Runtime) { - let mut from = rt.cli() +fn remove_linking(rt: &Runtime) -> Result<()> { + let mut from : FileLockEntry = rt.cli() .subcommand_matches("remove") .unwrap() // safe, we know there is an "remove" subcommand .value_of("from") .map(PathBuf::from) - .map(|id| { - rt.store() - .get(id) - .map_err_trace_exit_unwrap() - .ok_or_else(|| warn_exit("No 'from' entry", 1)) - .unwrap() // safe by line above - }) - .unwrap(); + .and_then(|id| rt.store().get(id).transpose()) + .ok_or_else(|| err_msg("No 'from' entry"))??; rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + .ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter() - .for_each(|id| match rt.store().get(id.clone()) { - Err(e) => trace_error(&e), - Ok(Some(mut to_entry)) => { - to_entry - .remove_link(&mut from) - .map_err_trace_exit_unwrap(); - - rt.report_touched(to_entry.get_location()).unwrap_or_exit(); + .map(|id| match rt.store().get(id.clone())? { + Some(mut to_entry) => { + to_entry.remove_link(&mut from)?; + rt.report_touched(to_entry.get_location()).map_err(Error::from) }, - Ok(None) => { + + None => { // looks like this is not an entry, but a filesystem URI and therefor an // external link...? if id.local().is_file() { - let pb = id.local().to_str().unwrap_or_else(|| { - warn!("Not StoreId and not a Path: {}", id); - ::std::process::exit(1); - }); - let url = Url::parse(pb).unwrap_or_else(|e| { - error!("Error parsing URL: {:?}", e); - ::std::process::exit(1); - }); - from.remove_url(rt.store(), url).map_err_trace_exit_unwrap(); + let pb = id.local().to_str().ok_or_else(|| format_err!("Not StoreId and not a Path: {}", id))?; + let url = Url::parse(pb).map_err(|e| format_err!("Error parsing URL: {:?}", e))?; + from.remove_url(rt.store(), url)?; info!("Ok: {}", id); + Ok(()) } else { - warn!("Entry not found: {:?}", id); + Err(format_err!("Entry not found: {:?}", id)) } } - }); - - rt.report_touched(from.get_location()).unwrap_or_exit(); -} - -fn unlink(rt: &Runtime) { - rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); }) - .into_iter() - .for_each(|id| { - rt.store() - .get(id.clone()) - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - warn!("No entry for {}", id); - ::std::process::exit(1) - }) - .unlink(rt.store()) - .map_err_trace_exit_unwrap(); + .collect::>>()?; - rt.report_touched(&id).unwrap_or_exit(); - }); + rt.report_touched(from.get_location()).map_err(Error::from) } -fn list_linkings(rt: &Runtime) { +fn unlink(rt: &Runtime) -> Result<()> { + rt + .ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? + .into_iter() + .map(|id| { + rt.store() + .get(id.clone())? + .ok_or_else(|| format_err!("No entry for {}", id))? + .unlink(rt.store())?; + + rt.report_touched(&id).map_err(Error::from) + }) + .collect() +} + +fn list_linkings(rt: &Runtime) -> Result<()> { let cmd = rt.cli() .subcommand_matches("list") .unwrap(); // safed by clap @@ -304,70 +244,50 @@ fn list_linkings(rt: &Runtime) { let mut tab = ::prettytable::Table::new(); tab.set_titles(row!["#", "Link"]); - rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + rt.ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter() - .for_each(|id| { - match rt.store().get(id.clone()) { - Ok(Some(entry)) => { - for (i, link) in entry.links().map_err_trace_exit_unwrap().enumerate() { - let link = link - .to_str() - .map_warn_err(|e| format!("Failed to convert StoreId to string: {:?}", e)) - .ok(); + .map(|id| { + let entry = rt.store().get(id.clone())?.ok_or_else(|| format_err!("Not found: {}", id))?; - if let Some(link) = link { - if list_plain { - writeln!(rt.stdout(), "{: <3}: {}", i, link) - .to_exit_code() - .unwrap_or_exit(); - } else { - tab.add_row(row![i, link]); - } - } - } + for (i, link) in entry.links()?.enumerate() { + let link = link.to_str()?; - if list_externals { - entry.get_urls(rt.store()) - .map_err_trace_exit_unwrap() - .enumerate() - .for_each(|(i, link)| { - let link = link - .map_err_trace_exit_unwrap() - .into_string(); - - if list_plain { - writeln!(rt.stdout(), "{: <3}: {}", i, link) - .to_exit_code() - .unwrap_or_exit(); - } else { - tab.add_row(row![i, link]); - } - }) - } - - rt.report_touched(entry.get_location()).unwrap_or_exit(); - - }, - Ok(None) => warn!("Not found: {}", id), - Err(e) => trace_error(&e), + if list_plain { + writeln!(rt.stdout(), "{: <3}: {}", i, link)?; + } else { + tab.add_row(row![i, link]); + } } - rt.report_touched(&id).unwrap_or_exit(); - }); + if list_externals { + entry.get_urls(rt.store())? + .enumerate() + .map(|(i, link)| { + let link = link?.into_string(); + + if list_plain { + writeln!(rt.stdout(), "{: <3}: {}", i, link)?; + } else { + tab.add_row(row![i, link]); + } + + Ok(()) + }) + .collect::>>()?; + } + + rt.report_touched(entry.get_location()).map_err(Error::from) + }) + .collect::>>()?; if !list_plain { let out = rt.stdout(); let mut lock = out.lock(); - tab.print(&mut lock) - .to_exit_code() - .unwrap_or_exit(); + tab.print(&mut lock)?; } + + Ok(()) } #[cfg(test)] @@ -449,7 +369,7 @@ mod tests { debug!("Entries created"); - link_from_to(&rt, "test1", vec!["test2"].into_iter()); + link_from_to(&rt, "test1", vec!["test2"].into_iter()).unwrap(); debug!("Linking done"); @@ -480,7 +400,7 @@ mod tests { debug!("Test entries created"); - link_from_to(&rt, "test1", vec!["test2"].into_iter()); + link_from_to(&rt, "test1", vec!["test2"].into_iter()).unwrap(); debug!("Linking done"); @@ -509,8 +429,8 @@ mod tests { debug!("Test entries created"); - link_from_to(&rt, "test1", vec!["test2"].into_iter()); - link_from_to(&rt, "test1", vec!["test2"].into_iter()); + link_from_to(&rt, "test1", vec!["test2"].into_iter()).unwrap(); + link_from_to(&rt, "test1", vec!["test2"].into_iter()).unwrap(); debug!("Linking done"); @@ -540,8 +460,8 @@ mod tests { debug!("Test entries created"); - link_from_to(&rt, "test1", vec!["test2", "test3"].into_iter()); - link_from_to(&rt, "test1", vec!["test2", "test3"].into_iter()); + link_from_to(&rt, "test1", vec!["test2", "test3"].into_iter()).unwrap(); + link_from_to(&rt, "test1", vec!["test2", "test3"].into_iter()).unwrap(); debug!("Linking done"); @@ -576,14 +496,14 @@ mod tests { debug!("Test entries created"); - link_from_to(&rt, "test1", vec!["test2"].into_iter()); + link_from_to(&rt, "test1", vec!["test2"].into_iter()).unwrap(); debug!("Linking done"); let rt = reset_test_runtime(vec!["remove", "test1", "test2"], rt) .unwrap(); - remove_linking(&rt); + remove_linking(&rt).unwrap(); debug!("Linking removed"); @@ -613,14 +533,14 @@ mod tests { debug!("Test entries created"); - link_from_to(&rt, "test1", vec!["test2", "test3"].into_iter()); + link_from_to(&rt, "test1", vec!["test2", "test3"].into_iter()).unwrap(); debug!("linking done"); let rt = reset_test_runtime(vec!["remove", "test1", "test2", "test3"], rt) .unwrap(); - remove_linking(&rt); + remove_linking(&rt).unwrap(); debug!("linking removed"); From ab39aa935337fa08219138364c440275cd989852 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 18/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-markdown/Cargo.toml | 1 + bin/core/imag-markdown/src/lib.rs | 46 ++++++++++++++----------------- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/bin/core/imag-markdown/Cargo.toml b/bin/core/imag-markdown/Cargo.toml index e35d431a..21311975 100644 --- a/bin/core/imag-markdown/Cargo.toml +++ b/bin/core/imag-markdown/Cargo.toml @@ -23,6 +23,7 @@ maintenance = { status = "actively-developed" } [dependencies] log = "0.4.6" failure = "0.1.5" +resiter = "0.3.0" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-markdown/src/lib.rs b/bin/core/imag-markdown/src/lib.rs index 2250fe41..1d88e4c2 100644 --- a/bin/core/imag-markdown/src/lib.rs +++ b/bin/core/imag-markdown/src/lib.rs @@ -37,6 +37,7 @@ extern crate clap; #[macro_use] extern crate log; extern crate failure; +extern crate resiter; extern crate libimagerror; extern crate libimagrt; @@ -47,13 +48,14 @@ use std::io::Write; use failure::Error; use failure::err_msg; use failure::Fallible as Result; +use resiter::AndThen; +use resiter::Map; use clap::App; -use libimagerror::trace::MapErrTrace; -use libimagerror::iter::TraceIterator; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; use libimagstore::iter::get::StoreIdGetIteratorExtension; +use crate::libimagerror::iter::IterInnerOkOrElse; mod ui; @@ -69,40 +71,32 @@ impl ImagApplication for ImagMarkdown { let mut outlock = out.lock(); let iter = rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + .ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter() .map(Ok) .into_get_iter(rt.store()) - .trace_unwrap_exit() - .map(|ofle| ofle.ok_or_else(|| { - err_msg("Entry does not exist but is in store. This is a BUG, please report!") - })) - .trace_unwrap_exit(); + .map_inner_ok_or_else(|| err_msg("Entry does not exist but is in store. This is a BUG, please report!")); if only_links { - iter.map(|fle| libimagentrymarkdown::link::extract_links(fle.get_content())) - .for_each(|links| { - links.iter().for_each(|link| { - writeln!(outlock, "{title}: {link}", title = link.title, link = link.link) - .map_err(Error::from) - .map_err_trace_exit_unwrap(); - }) + debug!("Printing only links"); + iter.map_ok(|fle| libimagentrymarkdown::link::extract_links(fle.get_content())) + .and_then_ok(|links| { + links.iter() + .map(|link| { + writeln!(outlock, "{title}: {link}", title = link.title, link = link.link).map_err(Error::from) + }) + .collect() }) + .collect() } else { - iter.map(|fle| libimagentrymarkdown::html::to_html(fle.get_content())) - .trace_unwrap_exit() - .for_each(|html| { - writeln!(outlock, "{}", html).map_err(Error::from).map_err_trace_exit_unwrap(); + iter.and_then_ok(|fle| libimagentrymarkdown::html::to_html(fle.get_content())) + .and_then_ok(|html| { + writeln!(outlock, "{}", html).map_err(Error::from).map_err(Error::from) }) + .collect() } - - Ok(()) } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { From e740b2faaa4c27ee16a74a1efbc61a1fdb6a9723 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 19/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-mv/Cargo.toml | 2 +- bin/core/imag-mv/src/lib.rs | 92 +++++++++++++------------------------ 2 files changed, 33 insertions(+), 61 deletions(-) diff --git a/bin/core/imag-mv/Cargo.toml b/bin/core/imag-mv/Cargo.toml index 41df6adc..475ca01c 100644 --- a/bin/core/imag-mv/Cargo.toml +++ b/bin/core/imag-mv/Cargo.toml @@ -20,7 +20,7 @@ is-it-maintained-open-issues = { repository = "matthiasbeyer/imag" } maintenance = { status = "actively-developed" } [dependencies] -log = "0.4.6" +log = "0.4.6" failure = "0.1.5" libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-mv/src/lib.rs b/bin/core/imag-mv/src/lib.rs index 4107e7b3..e19cb6b0 100644 --- a/bin/core/imag-mv/src/lib.rs +++ b/bin/core/imag-mv/src/lib.rs @@ -35,33 +35,29 @@ )] #[macro_use] extern crate log; +#[macro_use] extern crate failure; extern crate clap; -extern crate failure; extern crate libimagrt; extern crate libimagstore; extern crate libimagerror; extern crate libimagentrylink; -use std::process::exit; - mod ui; use std::path::PathBuf; -use std::result::Result as RResult; use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; -use libimagerror::trace::MapErrTrace; -use libimagerror::iter::TraceIterator; -use libimagerror::exit::ExitUnwrap; use libimagstore::storeid::StoreId; use libimagstore::store::Store; use libimagstore::store::FileLockEntry; use libimagentrylink::linkable::Linkable; use libimagstore::iter::get::StoreIdGetIteratorExtension; +use libimagerror::iter::IterInnerOkOrElse; use failure::Fallible as Result; +use failure::err_msg; use clap::App; @@ -77,72 +73,50 @@ impl ImagApplication for ImagMv { .value_of("source") .map(PathBuf::from) .map(StoreId::new) - .unwrap() // unwrap safe by clap - .map_err_trace_exit_unwrap(); + .unwrap()?; // unwrap safe by clap let destname = rt .cli() .value_of("dest") .map(PathBuf::from) .map(StoreId::new) - .unwrap() // unwrap safe by clap - .map_err_trace_exit_unwrap(); + .unwrap()?; // unwrap safe by clap // remove links to entry, and re-add them later - let mut linked_entries = { - rt.store() - .get(sourcename.clone()) - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("Funny things happened: Entry moved to destination did not fail, but entry does not exist"); - exit(1) - }) - .links() - .map_err_trace_exit_unwrap() - .map(|link| Ok(link.get_store_id().clone()) as RResult<_, _>) - .into_get_iter(rt.store()) - .trace_unwrap_exit() - .map(|e| { - e.unwrap_or_else(|| { - error!("Linked entry does not exist"); - exit(1) - }) - }) - .collect::>() - }; + let mut linked_entries = rt.store() + .get(sourcename.clone())? + .ok_or_else(|| format_err!("Entry does not exist: {}", sourcename))? + .links()? + .map(|link| link.get_store_id().clone()) + .map(Ok) + .into_get_iter(rt.store()) + .map_inner_ok_or_else(|| err_msg("Linked entry does not exist")) + .collect::>>()?; { // remove links to linked entries from source let mut entry = rt .store() - .get(sourcename.clone()) - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("Source Entry does not exist"); - exit(1) - }); + .get(sourcename.clone())? + .ok_or_else(|| err_msg("Source Entry does not exist"))?; for link in linked_entries.iter_mut() { - let _ = entry.remove_link(link).map_err_trace_exit_unwrap(); + entry.remove_link(link)?; } } - let _ = rt - .store() - .move_by_id(sourcename.clone(), destname.clone()) - .map_err(|e| { // on error, re-add links - debug!("Re-adding links to source entry because moving failed"); - relink(rt.store(), sourcename.clone(), &mut linked_entries); - e - }) - .map_err_trace_exit_unwrap(); + if let Err(e) = rt.store().move_by_id(sourcename.clone(), destname.clone()) { + debug!("Re-adding links to source entry because moving failed"); + relink(rt.store(), sourcename.clone(), &mut linked_entries)?; - let _ = rt.report_touched(&destname).unwrap_or_exit(); + return Err(e); + } + + rt.report_touched(&destname)?; // re-add links to moved entry - relink(rt.store(), destname, &mut linked_entries); + relink(rt.store(), destname, &mut linked_entries)?; info!("Ok."); - Ok(()) } @@ -165,17 +139,15 @@ impl ImagApplication for ImagMv { -fn relink<'a>(store: &'a Store, target: StoreId, linked_entries: &mut Vec>) { +fn relink<'a>(store: &'a Store, target: StoreId, linked_entries: &mut Vec>) -> Result<()> { let mut entry = store - .get(target) - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("Funny things happened: Entry moved to destination did not fail, but entry does not exist"); - exit(1) - }); - + .get(target)? + .ok_or_else(|| err_msg("Funny things happened: Entry moved to destination did not fail, but entry does not exist"))?; for mut link in linked_entries { - let _ = entry.add_link(&mut link).map_err_trace_exit_unwrap(); + let _ = entry.add_link(&mut link)?; } + + Ok(()) } + From e67f5374bef4ec692cb2048ecb83276db0ba1926 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 20/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-ref/src/lib.rs | 144 ++++++++++++++--------------------- 1 file changed, 56 insertions(+), 88 deletions(-) diff --git a/bin/core/imag-ref/src/lib.rs b/bin/core/imag-ref/src/lib.rs index 7cbcb251..c5b5f982 100644 --- a/bin/core/imag-ref/src/lib.rs +++ b/bin/core/imag-ref/src/lib.rs @@ -47,22 +47,20 @@ extern crate libimagutil; mod ui; -use std::process::exit; use std::io::Write; -use failure::Error; use failure::Fallible as Result; +use failure::Error; +use failure::err_msg; use clap::App; -use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; use libimagrt::application::ImagApplication; use libimagrt::runtime::Runtime; use libimagentryref::reference::Ref; -use libimagentryref::reference::MutRef; use libimagentryref::reference::RefFassade; use libimagentryref::hasher::default::DefaultHasher; use libimagentryref::util::get_ref_config; +use libimagentryref::reference::MutRef; /// Marker enum for implementing ImagApplication on /// @@ -80,15 +78,16 @@ impl ImagApplication for ImagRef { "list-dead" => list_dead(&rt), other => { debug!("Unknown command"); - let _ = rt.handle_unknown_subcommand("imag-ref", other, rt.cli()) - .map_err_trace_exit_unwrap() - .code() - .map(::std::process::exit); + if rt.handle_unknown_subcommand("imag-ref", other, rt.cli())?.success() { + Ok(()) + } else { + Err(format_err!("Subcommand failed")) + } }, } - }; - - Ok(()) + } else { + Ok(()) + } } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { @@ -108,23 +107,18 @@ impl ImagApplication for ImagRef { } } -fn deref(rt: &Runtime) { +fn deref(rt: &Runtime) -> Result<()> { let cmd = rt.cli().subcommand_matches("deref").unwrap(); let basepath = cmd.value_of("override-basepath"); - let cfg = get_ref_config(&rt, "imag-ref").map_err_trace_exit_unwrap(); + let cfg = get_ref_config(&rt, "imag-ref")?; let out = rt.stdout(); let mut outlock = out.lock(); - rt - .ids::<::ui::PathProvider>() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + rt.ids::<::ui::PathProvider>()? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter() - .for_each(|id| { - match rt.store().get(id.clone()).map_err_trace_exit_unwrap() { + .map(|id| { + match rt.store().get(id.clone())? { Some(entry) => { let r_entry = entry.as_ref_with_hasher::(); @@ -132,89 +126,63 @@ fn deref(rt: &Runtime) { r_entry.get_path_with_basepath_setting(&cfg, alternative_basepath) } else { r_entry.get_path(&cfg) - } - .map_err_trace_exit_unwrap() + }? .to_str() - .ok_or_else(|| ::libimagerror::errors::ErrorMsg::UTF8Error) - .map_err(Error::from) - .and_then(|s| writeln!(outlock, "{}", s).map_err(Error::from)) - .map_err_trace_exit_unwrap(); + .ok_or_else(|| Error::from(::libimagerror::errors::ErrorMsg::UTF8Error)) + .and_then(|s| writeln!(outlock, "{}", s).map_err(Error::from))?; - rt.report_touched(&id).unwrap_or_exit(); - }, - None => { - error!("No entry for id '{}' found", id); - exit(1) + rt.report_touched(&id).map_err(Error::from) }, + None => Err(format_err!("No entry for id '{}' found", id)) } - }); + }) + .collect() } -fn remove(rt: &Runtime) { +fn remove(rt: &Runtime) -> Result<()> { use libimaginteraction::ask::ask_bool; - let cmd = rt.cli().subcommand_matches("remove").unwrap(); - let yes = cmd.is_present("yes"); - - let mut input = rt.stdin().unwrap_or_else(|| { - error!("No input stream. Cannot ask for permission"); - exit(1); - }); - + let cmd = rt.cli().subcommand_matches("remove").unwrap(); + let yes = cmd.is_present("yes"); + let mut input = rt.stdin().ok_or_else(|| err_msg("No input stream. Cannot ask for permission"))?; let mut output = rt.stdout(); - rt - .ids::<::ui::PathProvider>() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + rt.ids::<::ui::PathProvider>()? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter() - .for_each(|id| { - match rt.store().get(id.clone()).map_err_trace_exit_unwrap() { + .map(|id| { + match rt.store().get(id.clone())? { + None => Err(format_err!("No entry for id '{}' found", id)), Some(mut entry) => { - if yes || - ask_bool(&format!("Delete ref from entry '{}'", id), None, &mut input, &mut output) - .map_err_trace_exit_unwrap() - { - entry.as_ref_with_hasher_mut::() - .remove_ref() - .map_err_trace_exit_unwrap(); + if yes || ask_bool(&format!("Delete ref from entry '{}'", id), None, &mut input, &mut output)? { + entry.as_ref_with_hasher_mut::().remove_ref() } else { info!("Aborted"); + Ok(()) } }, - None => { - error!("No entry for id '{}' found", id); - exit(1) - }, } - }); + }) + .collect() } -fn list_dead(rt: &Runtime) { - let cfg = get_ref_config(&rt, "imag-ref").map_err_trace_exit_unwrap(); +fn list_dead(rt: &Runtime) -> Result<()> { + let cfg = get_ref_config(&rt, "imag-ref")?; let cmd = rt.cli().subcommand_matches("list-dead").unwrap(); // safe by main() let list_path = cmd.is_present("list-dead-pathes"); let list_id = cmd.is_present("list-dead-ids"); let mut output = rt.stdout(); - rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + rt.ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter() - .for_each(|id| { - match rt.store().get(id.clone()).map_err_trace_exit_unwrap() { + .map(|id| { + match rt.store().get(id.clone())? { Some(entry) => { let entry_ref = entry.as_ref_with_hasher::(); - if entry_ref.is_ref().map_err_trace_exit_unwrap() { // we only care if the entry is a ref - let entry_path = entry_ref.get_path(&cfg).map_err_trace_exit_unwrap(); + if entry_ref.is_ref()? { // we only care if the entry is a ref + let entry_path = entry_ref.get_path(&cfg)?; if !entry_path.exists() { if list_id { @@ -223,24 +191,24 @@ fn list_dead(rt: &Runtime) { writeln!(output, "{}", entry_path.display()) } else { unimplemented!() - } - .map_err(Error::from) - .map_err_trace_exit_unwrap(); + }?; - rt.report_touched(entry.get_location()).unwrap_or_exit(); + rt.report_touched(entry.get_location()).map_err(Error::from) + } else { + Ok(()) } + } else { + Ok(()) } } - None => { - error!("Does not exist: {}", id.local().display()); - exit(1) - } + None => Err(format_err!("Does not exist: {}", id.local().display())), } - }); + }) + .collect() } -fn create(_rt: &Runtime) { +fn create(_rt: &Runtime) -> Result<()> { unimplemented!() } From 58e303116d102162dfa61d6bbca6c4a0b00768cf Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 21/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-store/Cargo.toml | 1 + bin/core/imag-store/src/create.rs | 31 ++++++------------- bin/core/imag-store/src/delete.rs | 21 ++++++------- bin/core/imag-store/src/get.rs | 20 ++++++------ bin/core/imag-store/src/lib.rs | 27 ++++++++-------- bin/core/imag-store/src/retrieve.rs | 48 +++++++++++------------------ bin/core/imag-store/src/update.rs | 17 +++++----- bin/core/imag-store/src/verify.rs | 31 ++++++++++--------- 8 files changed, 88 insertions(+), 108 deletions(-) diff --git a/bin/core/imag-store/Cargo.toml b/bin/core/imag-store/Cargo.toml index a964363c..0624069d 100644 --- a/bin/core/imag-store/Cargo.toml +++ b/bin/core/imag-store/Cargo.toml @@ -23,6 +23,7 @@ maintenance = { status = "actively-developed" } log = "0.4.6" toml = "0.5.1" failure = "0.1.5" +resiter = "0.3.0" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore", features = ["verify"] } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } diff --git a/bin/core/imag-store/src/create.rs b/bin/core/imag-store/src/create.rs index c4a8ad14..a8989a28 100644 --- a/bin/core/imag-store/src/create.rs +++ b/bin/core/imag-store/src/create.rs @@ -17,34 +17,32 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // +use std::ops::DerefMut; use std::path::PathBuf; use std::io::stdin; use std::fs::OpenOptions; use std::io::Read; -use std::ops::DerefMut; use clap::ArgMatches; use toml::Value; use failure::Fallible as Result; +use failure::Error; use failure::err_msg; use libimagrt::runtime::Runtime; use libimagstore::store::Entry; use libimagstore::storeid::StoreId; -use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; -use libimagutil::debug_result::*; use crate::util::build_toml_header; -pub fn create(rt: &Runtime) { +pub fn create(rt: &Runtime) -> Result<()> { let scmd = rt.cli().subcommand_matches("create").unwrap(); debug!("Found 'create' subcommand..."); // unwrap is safe as value is required let path = scmd.value_of("path").unwrap(); let path = PathBuf::from(path); - let path = StoreId::new(path).map_err_trace_exit_unwrap(); + let path = StoreId::new(path)?; debug!("path = {:?}", path); @@ -55,15 +53,13 @@ pub fn create(rt: &Runtime) { .or_else(|_| create_with_content_and_header(rt, &path, String::new(), - Entry::default_header())) + Entry::default_header()))?; } else { debug!("Creating entry"); - create_with_content_and_header(rt, &path, String::new(), - Entry::default_header()) + create_with_content_and_header(rt, &path, String::new(), Entry::default_header())?; } - .map_err_trace_exit_unwrap(); - rt.report_touched(&path).unwrap_or_exit(); + rt.report_touched(&path).map_err(Error::from) } fn create_from_cli_spec(rt: &Runtime, matches: &ArgMatches, path: &StoreId) -> Result<()> { @@ -99,19 +95,13 @@ fn create_from_source(rt: &Runtime, matches: &ArgMatches, path: &StoreId) -> Res debug!("Content with len = {}", content.len()); Entry::from_str(path.clone(), &content[..]) - .map_dbg_err(|e| format!("Error building entry: {:?}", e)) .and_then(|new_e| { - let r = rt.store() + rt.store() .create(path.clone()) - .map_dbg_err(|e| format!("Error in Store::create(): {:?}", e)) .map(|mut old_e| { *old_e.deref_mut() = new_e; - }); - - debug!("Entry build"); - r + }) }) - .map_dbg_err(|e| format!("Error storing entry: {:?}", e)) } fn create_with_content_and_header(rt: &Runtime, @@ -122,7 +112,6 @@ fn create_with_content_and_header(rt: &Runtime, debug!("Creating entry with content at {:?}", path); rt.store() .create(path.clone()) - .map_dbg_err(|e| format!("Error in Store::create(): {:?}", e)) .map(|mut element| { { let e_content = element.get_content_mut(); @@ -177,7 +166,7 @@ mod tests { let test_name = "test_create_simple"; let rt = generate_test_runtime(vec!["create", "test_create_simple"]).unwrap(); - create(&rt); + create(&rt).unwrap(); let e = rt.store().get(PathBuf::from(test_name)); assert!(e.is_ok()); diff --git a/bin/core/imag-store/src/delete.rs b/bin/core/imag-store/src/delete.rs index e361d535..226e08a0 100644 --- a/bin/core/imag-store/src/delete.rs +++ b/bin/core/imag-store/src/delete.rs @@ -19,22 +19,19 @@ use std::path::PathBuf; -use libimagrt::runtime::Runtime; -use libimagerror::trace::MapErrTrace; -use libimagstore::storeid::StoreId; -use libimagutil::warn_result::*; +use failure::Fallible as Result; -pub fn delete(rt: &Runtime) { +use libimagrt::runtime::Runtime; +use libimagstore::storeid::StoreId; + +pub fn delete(rt: &Runtime) -> Result<()> { let scmd = rt.cli().subcommand_matches("delete").unwrap(); let id = scmd.value_of("id").unwrap(); // safe by clap let path = PathBuf::from(id); - let path = StoreId::new(path).map_err_trace_exit_unwrap(); + let path = StoreId::new(path)?; debug!("Deleting file at {:?}", id); - rt.store() - .delete(path) - .map_warn_err(|e| format!("Error: {:?}", e)) - .map_err_trace_exit_unwrap(); + rt.store().delete(path) } #[cfg(test)] @@ -59,11 +56,11 @@ mod tests { let test_name = "test_create_simple"; let rt = generate_test_runtime(vec!["create", "test_create_simple"]).unwrap(); - create(&rt); + create(&rt).unwrap(); let rt = reset_test_runtime(vec!["delete", "test_create_simple"], rt).unwrap(); - delete(&rt); + delete(&rt).unwrap(); let e = rt.store().get(PathBuf::from(test_name)); assert!(e.is_ok()); diff --git a/bin/core/imag-store/src/get.rs b/bin/core/imag-store/src/get.rs index 21115398..b47fabc3 100644 --- a/bin/core/imag-store/src/get.rs +++ b/bin/core/imag-store/src/get.rs @@ -19,27 +19,29 @@ use std::path::PathBuf; +use failure::Fallible as Result; +use failure::Error; +use failure::err_msg; + use libimagrt::runtime::Runtime; -use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; use libimagstore::storeid::StoreId; use crate::retrieve::print_entry; -pub fn get(rt: &Runtime) { +pub fn get(rt: &Runtime) -> Result<()> { let scmd = rt.cli().subcommand_matches("get").unwrap(); let id = scmd.value_of("id").unwrap(); // safe by clap let path = PathBuf::from(id); - let path = StoreId::new(path).map_err_trace_exit_unwrap(); + let path = StoreId::new(path)?; debug!("path = {:?}", path); - match rt.store().get(path.clone()).map_err_trace_exit_unwrap() { + match rt.store().get(path.clone())? { + None => Err(err_msg("No entry found")), Some(entry) => { - print_entry(rt, scmd, entry); - rt.report_touched(&path).unwrap_or_exit(); + print_entry(rt, scmd, entry)?; + rt.report_touched(&path).map_err(Error::from) }, - None => info!("No entry found"), - }; + } } diff --git a/bin/core/imag-store/src/lib.rs b/bin/core/imag-store/src/lib.rs index 1936ecef..53e023ed 100644 --- a/bin/core/imag-store/src/lib.rs +++ b/bin/core/imag-store/src/lib.rs @@ -37,8 +37,9 @@ extern crate clap; #[macro_use] extern crate log; extern crate toml; +extern crate resiter; #[cfg(test)] extern crate toml_query; -extern crate failure; +#[macro_use] extern crate failure; extern crate libimagrt; extern crate libimagstore; @@ -53,7 +54,9 @@ extern crate libimagutil; use libimagrt::application::ImagApplication; use libimagrt::runtime::Runtime; -use libimagerror::trace::MapErrTrace; + +use failure::Fallible as Result; +use failure::err_msg; mod create; mod delete; @@ -66,7 +69,6 @@ mod util; use std::ops::Deref; -use failure::Fallible as Result; use clap::App; use crate::create::create; @@ -83,9 +85,7 @@ use crate::verify::verify; pub enum ImagStore {} impl ImagApplication for ImagStore { fn run(rt: Runtime) -> Result<()> { - let command = rt.cli().subcommand_name().map(String::from); - - if let Some(command) = command { + if let Some(command) = rt.cli().subcommand_name() { debug!("Call: {}", command); match command.deref() { "create" => create(&rt), @@ -96,17 +96,16 @@ impl ImagApplication for ImagStore { "verify" => verify(&rt), other => { debug!("Unknown command"); - let _ = rt.handle_unknown_subcommand("imag-store", other, rt.cli()) - .map_err_trace_exit_unwrap() - .code() - .map(::std::process::exit); + if rt.handle_unknown_subcommand("imag-store", other, rt.cli())?.success() { + Ok(()) + } else { + Err(format_err!("Subcommand failed")) + } }, - }; + } } else { - debug!("No command"); + Err(err_msg("No command")) } - - Ok(()) } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { diff --git a/bin/core/imag-store/src/retrieve.rs b/bin/core/imag-store/src/retrieve.rs index a84d39d7..7ae516d7 100644 --- a/bin/core/imag-store/src/retrieve.rs +++ b/bin/core/imag-store/src/retrieve.rs @@ -20,41 +20,32 @@ use std::path::PathBuf; use std::io::Write; +use failure::Fallible as Result; +use failure::Error; use clap::ArgMatches; use libimagstore::store::FileLockEntry; use libimagstore::storeid::StoreId; use libimagrt::runtime::Runtime; -use libimagerror::trace::MapErrTrace; -use libimagerror::io::ToExitCode; -use libimagerror::exit::ExitUnwrap; -use libimagutil::debug_result::*; -pub fn retrieve(rt: &Runtime) { - if let Some(scmd) = rt.cli().subcommand_matches("retrieve") { - // unwrap() is safe as arg is required - let id = scmd.value_of("id").unwrap(); - let path = PathBuf::from(id); - let path = StoreId::new(path).map_err_trace_exit_unwrap(); - debug!("path = {:?}", path); +pub fn retrieve(rt: &Runtime) -> Result<()> { + let scmd = rt.cli().subcommand_matches("retrieve").unwrap(); + let id = scmd.value_of("id").unwrap(); + let path = PathBuf::from(id); + let path = StoreId::new(path)?; + debug!("path = {:?}", path); - rt.store() - .retrieve(path.clone()) - .map(|e| print_entry(rt, scmd, e)) - .map_dbg_str("No entry") - .map_dbg(|e| format!("{:?}", e)) - .map_err_trace_exit_unwrap(); + rt.store() + .retrieve(path.clone()) + .and_then(|e| print_entry(rt, scmd, e))?; - rt.report_touched(&path).unwrap_or_exit(); - } + rt.report_touched(&path).map_err(Error::from) } -pub fn print_entry(rt: &Runtime, scmd: &ArgMatches, e: FileLockEntry) { +pub fn print_entry(rt: &Runtime, scmd: &ArgMatches, e: FileLockEntry) -> Result<()> { if do_print_raw(scmd) { debug!("Printing raw content..."); - writeln!(rt.stdout(), "{}", e.to_str().map_err_trace_exit_unwrap()) - .to_exit_code() - .unwrap_or_exit(); + writeln!(rt.stdout(), "{}", e.to_str()?)?; } else if do_filter(scmd) { debug!("Filtering..."); warn!("Filtering via header specs is currently now supported."); @@ -71,20 +62,17 @@ pub fn print_entry(rt: &Runtime, scmd: &ArgMatches, e: FileLockEntry) { unimplemented!() } else { debug!("Printing header as TOML..."); - writeln!(rt.stdout(), "{}", e.get_header()) - .to_exit_code() - .unwrap_or_exit(); + writeln!(rt.stdout(), "{}", e.get_header())?; } } if do_print_content(scmd) { debug!("Printing content..."); - writeln!(rt.stdout(), "{}", e.get_content()) - .to_exit_code() - .unwrap_or_exit(); + writeln!(rt.stdout(), "{}", e.get_content())?; } - } + + Ok(()) } fn do_print_header(m: &ArgMatches) -> bool { diff --git a/bin/core/imag-store/src/update.rs b/bin/core/imag-store/src/update.rs index fad5b3f7..8f2479fd 100644 --- a/bin/core/imag-store/src/update.rs +++ b/bin/core/imag-store/src/update.rs @@ -20,22 +20,23 @@ use std::ops::DerefMut; use std::path::PathBuf; +use failure::Fallible as Result; +use failure::Error; + use libimagrt::runtime::Runtime; -use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; use libimagstore::storeid::StoreId; use crate::util::build_toml_header; -pub fn update(rt: &Runtime) { +pub fn update(rt: &Runtime) -> Result<()> { let scmd = rt.cli().subcommand_matches("update").unwrap(); let id = scmd.value_of("id").unwrap(); // Safe by clap let path = PathBuf::from(id); - let path = StoreId::new(path).map_err_trace_exit_unwrap(); + let path = StoreId::new(path)?; - let _ = rt.store() + rt.store() .retrieve(path) - .map(|mut locked_e| { + .and_then(|mut locked_e| { { let e = locked_e.deref_mut(); @@ -48,7 +49,7 @@ pub fn update(rt: &Runtime) { debug!("New header set"); } - rt.report_touched(locked_e.get_location()).unwrap_or_exit(); - }); + rt.report_touched(locked_e.get_location()).map_err(Error::from) + }) } diff --git a/bin/core/imag-store/src/verify.rs b/bin/core/imag-store/src/verify.rs index a43ad908..bf4c19d7 100644 --- a/bin/core/imag-store/src/verify.rs +++ b/bin/core/imag-store/src/verify.rs @@ -19,26 +19,25 @@ use std::ops::Deref; +use failure::Fallible as Result; +use failure::err_msg; +use resiter::AndThen; + use libimagrt::runtime::Runtime; -use libimagutil::warn_exit::warn_exit; -use libimagerror::trace::MapErrTrace; -use libimagerror::exit::ExitUnwrap; -use libimagerror::iter::TraceIterator; +use libimagerror::iter::IterInnerOkOrElse; /// Verify the store. /// /// This function is not intended to be called by normal programs but only by `imag-store`. -pub fn verify(rt: &Runtime) { +pub fn verify(rt: &Runtime) -> Result<()> { info!("Header | Content length | Path"); info!("-------+----------------+-----"); let result = rt .store() - .entries() - .map_err_trace_exit_unwrap() + .entries()? .into_get_iter() - .trace_unwrap_exit() - .filter_map(|x| x) - .all(|fle| { + .map_inner_ok_or_else(|| err_msg("Did not find one entry")) + .and_then_ok(|fle| { let p = fle.get_location(); let content_len = fle.get_content().len(); let (verify, status) = if fle.verify().is_ok() { @@ -48,14 +47,18 @@ pub fn verify(rt: &Runtime) { }; info!("{: >6} | {: >14} | {:?}", verify, content_len, p.deref()); - rt.report_touched(fle.get_location()).unwrap_or_exit(); - status - }); + rt.report_touched(fle.get_location())?; + Ok(status) + }) + .collect::>>()? + .iter() + .all(|x| *x); if result { info!("Store seems to be fine"); + Ok(()) } else { - warn_exit("Store seems to be broken somehow", 1); + Err(err_msg("Store seems to be broken somehow")) } } From a3eccc0723472d17eabbb3644809302d6c931e08 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 19 Oct 2019 08:54:57 +0200 Subject: [PATCH 22/22] Remove calls to exit() and replace them with error propagation up to main() Signed-off-by: Matthias Beyer --- bin/core/imag-tag/src/lib.rs | 167 +++++++++++++++-------------------- 1 file changed, 70 insertions(+), 97 deletions(-) diff --git a/bin/core/imag-tag/src/lib.rs b/bin/core/imag-tag/src/lib.rs index 2e917884..1a2a2b8e 100644 --- a/bin/core/imag-tag/src/lib.rs +++ b/bin/core/imag-tag/src/lib.rs @@ -38,7 +38,7 @@ extern crate clap; #[macro_use] extern crate log; #[cfg(test)] extern crate toml; -extern crate failure; +#[macro_use] extern crate failure; extern crate libimagstore; extern crate libimagrt; @@ -60,19 +60,17 @@ extern crate env_logger; use std::io::Write; +use failure::Fallible as Result; +use failure::Error; +use failure::err_msg; + use libimagrt::runtime::Runtime; use libimagrt::application::ImagApplication; use libimagentrytag::tagable::Tagable; use libimagentrytag::tag::Tag; -use libimagerror::trace::trace_error; -use libimagerror::trace::MapErrTrace; -use libimagerror::io::ToExitCode; -use libimagerror::exit::ExitUnwrap; use libimagstore::storeid::StoreId; -use libimagutil::warn_exit::warn_exit; use clap::{App, ArgMatches}; -use failure::Fallible as Result; mod ui; @@ -84,43 +82,40 @@ mod ui; pub enum ImagTag {} impl ImagApplication for ImagTag { fn run(rt: Runtime) -> Result<()> { - let ids = rt - .ids::() - .map_err_trace_exit_unwrap() - .unwrap_or_else(|| { - error!("No ids supplied"); - ::std::process::exit(1); - }) + let ids = rt.ids::()? + .ok_or_else(|| err_msg("No ids supplied"))? .into_iter(); if let Some(name) = rt.cli().subcommand_name() { match name { - "list" => for id in ids { - list(id, &rt) - }, - "remove" => for id in ids { + "list" => ids.into_iter().map(|id| list(id, &rt)).collect(), + + "remove" => ids.into_iter().map(|id| { let add = None; - let rem = get_remove_tags(rt.cli()); + let rem = get_remove_tags(rt.cli())?; debug!("id = {:?}, add = {:?}, rem = {:?}", id, add, rem); - alter(&rt, id, add, rem); - }, - "add" => for id in ids { - let add = get_add_tags(rt.cli()); + alter(&rt, id, add, rem) + }).collect(), + + "add" => ids.into_iter().map(|id| { + let add = get_add_tags(rt.cli())?; let rem = None; debug!("id = {:?}, add = {:?}, rem = {:?}", id, add, rem); - alter(&rt, id, add, rem); - }, + alter(&rt, id, add, rem) + }).collect(), + other => { debug!("Unknown command"); - let _ = rt.handle_unknown_subcommand("imag-tag", other, rt.cli()) - .map_err_trace_exit_unwrap() - .code() - .map(::std::process::exit); + if rt.handle_unknown_subcommand("imag-tag", other, rt.cli())?.success() { + Ok(()) + } else { + Err(format_err!("Subcommand failed")) + } }, } + } else { + Ok(()) } - - Ok(()) } fn build_cli<'a>(app: App<'a, 'a>) -> App<'a, 'a> { @@ -140,63 +135,46 @@ impl ImagApplication for ImagTag { } } -fn alter(rt: &Runtime, path: StoreId, add: Option>, rem: Option>) { - match rt.store().get(path.clone()) { - Ok(Some(mut e)) => { +fn alter(rt: &Runtime, path: StoreId, add: Option>, rem: Option>) -> Result<()> { + match rt.store().get(path.clone())? { + Some(mut e) => { debug!("Entry header now = {:?}", e.get_header()); if let Some(tags) = add { debug!("Adding tags = '{:?}'", tags); - for tag in tags { + tags.into_iter().map(|tag| { debug!("Adding tag '{:?}'", tag); - if let Err(e) = e.add_tag(tag) { - trace_error(&e); - } else { - debug!("Adding tag worked"); - } - } + e.add_tag(tag) + }).collect::>>()?; } // it is okay to ignore a None here debug!("Entry header now = {:?}", e.get_header()); if let Some(tags) = rem { debug!("Removing tags = '{:?}'", tags); - for tag in tags { + tags.into_iter().map(|tag| { debug!("Removing tag '{:?}'", tag); - if let Err(e) = e.remove_tag(tag) { - trace_error(&e); - } - } + e.remove_tag(tag) + }).collect::>>()?; } // it is okay to ignore a None here debug!("Entry header now = {:?}", e.get_header()); - }, - Ok(None) => { + None => { info!("No entry found."); }, - - Err(e) => { - info!("No entry."); - trace_error(&e); - }, } - rt.report_touched(&path).unwrap_or_exit(); + rt.report_touched(&path).map_err(Error::from) } -fn list(path: StoreId, rt: &Runtime) { - let entry = match rt.store().get(path.clone()).map_err_trace_exit_unwrap() { - Some(e) => e, - None => warn_exit("No entry found.", 1), - }; - - let scmd = rt.cli().subcommand_matches("list").unwrap(); // safe, we checked in main() - - let json_out = scmd.is_present("json"); - let line_out = scmd.is_present("linewise"); - let sepp_out = scmd.is_present("sep"); +fn list(path: StoreId, rt: &Runtime) -> Result<()> { + let entry = rt.store().get(path.clone())?.ok_or_else(|| err_msg("No entry found"))?; + let scmd = rt.cli().subcommand_matches("list").unwrap(); // safe, we checked in main() + let json_out = scmd.is_present("json"); + let line_out = scmd.is_present("linewise"); + let sepp_out = scmd.is_present("sep"); let mut comm_out = scmd.is_present("commasep"); if !vec![json_out, line_out, comm_out, sepp_out].iter().any(|v| *v) { @@ -204,7 +182,7 @@ fn list(path: StoreId, rt: &Runtime) { comm_out = true; } - let tags = entry.get_tags().map_err_trace_exit_unwrap(); + let tags = entry.get_tags()?; if json_out { unimplemented!() @@ -212,53 +190,44 @@ fn list(path: StoreId, rt: &Runtime) { if line_out { for tag in &tags { - writeln!(rt.stdout(), "{}", tag) - .to_exit_code() - .unwrap_or_exit(); + writeln!(rt.stdout(), "{}", tag)?; } } if sepp_out { let sepp = scmd.value_of("sep").unwrap(); // we checked before - writeln!(rt.stdout(), "{}", tags.join(sepp)) - .to_exit_code() - .unwrap_or_exit(); + writeln!(rt.stdout(), "{}", tags.join(sepp))?; } if comm_out { - writeln!(rt.stdout(), "{}", tags.join(", ")) - .to_exit_code() - .unwrap_or_exit(); + writeln!(rt.stdout(), "{}", tags.join(", "))?; } - rt.report_touched(&path).unwrap_or_exit(); + rt.report_touched(&path).map_err(Error::from) } /// Get the tags which should be added from the commandline /// /// Returns none if the argument was not specified -fn get_add_tags(matches: &ArgMatches) -> Option> { +fn get_add_tags(matches: &ArgMatches) -> Result>> { retrieve_tags(matches, "add", "add-tags") } /// Get the tags which should be removed from the commandline /// /// Returns none if the argument was not specified -fn get_remove_tags(matches: &ArgMatches) -> Option> { +fn get_remove_tags(matches: &ArgMatches) -> Result>> { retrieve_tags(matches, "remove", "remove-tags") } -fn retrieve_tags(m: &ArgMatches, s: &'static str, v: &'static str) -> Option> { - Some(m +fn retrieve_tags(m: &ArgMatches, s: &'static str, v: &'static str) -> Result>> { + Ok(Some(m .subcommand_matches(s) - .unwrap_or_else(|| { - error!("Expected subcommand '{}', but was not specified", s); - ::std::process::exit(1) - }) + .ok_or_else(|| format_err!("Expected subcommand '{}', but was not specified", s))? .values_of(v) .unwrap() // enforced by clap .map(String::from) - .collect()) + .collect())) } #[cfg(test)] @@ -314,7 +283,7 @@ mod tests { } #[test] - fn test_tag_add_adds_tag() { + fn test_tag_add_adds_tag() -> Result<()> { setup_logging(); debug!("Generating runtime"); let name = "test-tag-add-adds-tags"; @@ -325,11 +294,11 @@ mod tests { let id = PathBuf::from(String::from(name)); debug!("Getting 'add' tags"); - let add = get_add_tags(rt.cli()); + let add = get_add_tags(rt.cli())?; debug!("Add-tags: {:?}", add); debug!("Altering things"); - alter(&rt, StoreId::new(id.clone()).unwrap(), add, None); + alter(&rt, StoreId::new(id.clone()).unwrap(), add, None)?; debug!("Altered"); let test_entry = rt.store().get(id).unwrap().unwrap(); @@ -343,10 +312,11 @@ mod tests { assert_ne!(*test_tags, tags_toml_value(vec![])); assert_eq!(*test_tags, tags_toml_value(vec!["foo"])); + Ok(()) } #[test] - fn test_tag_remove_removes_tag() { + fn test_tag_remove_removes_tag() -> Result<()> { setup_logging(); debug!("Generating runtime"); let name = "test-tag-remove-removes-tag"; @@ -360,21 +330,22 @@ mod tests { let add = Some(vec![ "foo".to_owned() ]); debug!("Getting 'remove' tags"); - let rem = get_remove_tags(rt.cli()); + let rem = get_remove_tags(rt.cli())?; debug!("Rem-tags: {:?}", rem); debug!("Altering things"); - alter(&rt, StoreId::new(id.clone()).unwrap(), add, rem); + alter(&rt, StoreId::new(id.clone()).unwrap(), add, rem)?; debug!("Altered"); let test_entry = rt.store().get(id).unwrap().unwrap(); let test_tags = get_entry_tags(&test_entry).unwrap().unwrap(); assert_eq!(*test_tags, tags_toml_value(vec![])); + Ok(()) } #[test] - fn test_tag_remove_removes_only_to_remove_tag() { + fn test_tag_remove_removes_only_to_remove_tag() -> Result<()> { setup_logging(); debug!("Generating runtime"); let name = "test-tag-remove-removes-only-to-remove-tag-doesnt-crash-on-nonexistent-tag"; @@ -388,21 +359,22 @@ mod tests { let add = Some(vec![ "foo".to_owned(), "bar".to_owned() ]); debug!("Getting 'remove' tags"); - let rem = get_remove_tags(rt.cli()); + let rem = get_remove_tags(rt.cli())?; debug!("Rem-tags: {:?}", rem); debug!("Altering things"); - alter(&rt, StoreId::new(id.clone()).unwrap(), add, rem); + alter(&rt, StoreId::new(id.clone()).unwrap(), add, rem)?; debug!("Altered"); let test_entry = rt.store().get(id).unwrap().unwrap(); let test_tags = get_entry_tags(&test_entry).unwrap().unwrap(); assert_eq!(*test_tags, tags_toml_value(vec!["bar"])); + Ok(()) } #[test] - fn test_tag_remove_removes_but_doesnt_crash_on_nonexistent_tag() { + fn test_tag_remove_removes_but_doesnt_crash_on_nonexistent_tag() -> Result<()> { setup_logging(); debug!("Generating runtime"); let name = "test-tag-remove-removes-but-doesnt-crash-on-nonexistent-tag"; @@ -416,17 +388,18 @@ mod tests { let add = Some(vec![ "foo".to_owned() ]); debug!("Getting 'remove' tags"); - let rem = get_remove_tags(rt.cli()); + let rem = get_remove_tags(rt.cli())?; debug!("Rem-tags: {:?}", rem); debug!("Altering things"); - alter(&rt, StoreId::new(id.clone()).unwrap(), add, rem); + alter(&rt, StoreId::new(id.clone()).unwrap(), add, rem)?; debug!("Altered"); let test_entry = rt.store().get(id).unwrap().unwrap(); let test_tags = get_entry_tags(&test_entry).unwrap().unwrap(); assert_eq!(*test_tags, tags_toml_value(vec![])); + Ok(()) } }