From ed457495c8c2e1f0d8caff919cd09fe238baa6b3 Mon Sep 17 00:00:00 2001 From: Leon Schuermann Date: Sat, 18 May 2019 02:42:38 +0200 Subject: [PATCH] Introduce proper error handling in IdPathProvider Prior to this change, the IdPathProvider implementation would be responsible for exiting the process on insufficient / wrong arguments. However, such error handling should be performed together with the business logic and not in CLI-parsing related code. This change introduces a clear separation: both parsing errors and insufficient id path arguments can now be return from inside the `get_ids`-method, and get passed up to the application logic to be handled. This change is reflected in all instances of IdPathProvider and their surrounding code. Signed-off-by: Leon Schuermann Signed-off-by: Matthias Beyer --- bin/core/imag-annotate/src/main.rs | 35 +++++++-- bin/core/imag-annotate/src/ui.rs | 64 +++++---------- bin/core/imag-category/Cargo.toml | 1 + bin/core/imag-category/src/main.rs | 22 +++++- bin/core/imag-category/src/ui.rs | 78 ++++++------------- bin/core/imag-edit/Cargo.toml | 1 + bin/core/imag-edit/src/main.rs | 10 ++- bin/core/imag-edit/src/ui.rs | 24 +++--- bin/core/imag-gps/src/main.rs | 29 ++++--- bin/core/imag-gps/src/ui.rs | 65 ++++------------ bin/core/imag-header/src/main.rs | 9 ++- bin/core/imag-header/src/ui.rs | 15 ++-- bin/core/imag-ids/src/main.rs | 10 ++- bin/core/imag-ids/src/ui.rs | 6 +- bin/core/imag-link/src/main.rs | 121 +++++++++++++++++------------ bin/core/imag-link/src/ui.rs | 74 ++++-------------- bin/core/imag-markdown/src/main.rs | 7 +- bin/core/imag-markdown/src/ui.rs | 21 +++-- bin/core/imag-ref/src/main.rs | 29 +++++-- bin/core/imag-ref/src/ui.rs | 72 +++++------------ bin/core/imag-tag/Cargo.toml | 2 +- bin/core/imag-tag/src/main.rs | 11 ++- bin/core/imag-tag/src/ui.rs | 14 ++-- bin/core/imag-view/src/main.rs | 7 +- bin/core/imag-view/src/ui.rs | 14 ++-- bin/core/imag/Cargo.toml | 1 + bin/domain/imag-mail/src/main.rs | 9 ++- bin/domain/imag-mail/src/ui.rs | 21 ++--- lib/core/libimagrt/src/runtime.rs | 17 ++-- 29 files changed, 378 insertions(+), 411 deletions(-) diff --git a/bin/core/imag-annotate/src/main.rs b/bin/core/imag-annotate/src/main.rs index f4ad2093..5904b345 100644 --- a/bin/core/imag-annotate/src/main.rs +++ b/bin/core/imag-annotate/src/main.rs @@ -96,8 +96,15 @@ fn main() { } fn add(rt: &Runtime) { - let scmd = rt.cli().subcommand_matches("add").unwrap(); // safed by main() - let mut ids = rt.ids::().map_err_trace_exit_unwrap().into_iter(); + 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); + }) + .into_iter(); if let Some(first) = ids.next() { let mut annotation = rt.store() @@ -141,10 +148,17 @@ fn add(rt: &Runtime) { } fn remove(rt: &Runtime) { - let scmd = rt.cli().subcommand_matches("remove").unwrap(); // safed by main() + 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(); + 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(); ids.into_iter().for_each(|id| { let mut entry = rt.store() @@ -179,9 +193,16 @@ fn remove(rt: &Runtime) { } fn list(rt: &Runtime) { - let scmd = rt.cli().subcommand_matches("list").unwrap(); // safed by clap + 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(); + let ids = rt + .ids::() + .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) + .into_iter(); if ids.len() != 0 { let _ = ids diff --git a/bin/core/imag-annotate/src/ui.rs b/bin/core/imag-annotate/src/ui.rs index 72074c08..2ad23262 100644 --- a/bin/core/imag-annotate/src/ui.rs +++ b/bin/core/imag-annotate/src/ui.rs @@ -24,7 +24,8 @@ use clap::{Arg, ArgMatches, App, SubCommand}; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; + +use failure::Fallible as Result; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { app @@ -97,53 +98,24 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { + fn get_ids(matches: &ArgMatches) -> Result>> { + fn get_id_paths(subm: &ArgMatches) -> Result>> { + subm.values_of("entry") + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() + } + match matches.subcommand() { - ("add", Some(subm)) => { - subm.values_of("entry") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - ("remove", Some(subm)) => { - subm.values_of("entry") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - ("list", Some(subm)) => { - subm.values_of("entry") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - + ("add", Some(subm)) => get_id_paths(subm), + ("remove", Some(subm)) => get_id_paths(subm), + ("list", Some(subm)) => get_id_paths(subm), (other, _) => { - error!("Not a known command: {}", other); - ::std::process::exit(1) + Err(format_err!("Not a known command: {}", other)) } } } diff --git a/bin/core/imag-category/Cargo.toml b/bin/core/imag-category/Cargo.toml index ad460881..5970ba66 100644 --- a/bin/core/imag-category/Cargo.toml +++ b/bin/core/imag-category/Cargo.toml @@ -23,6 +23,7 @@ maintenance = { status = "actively-developed" } log = "0.4.0" toml = "0.4" toml-query = "0.8" +failure = "0.1" 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/main.rs b/bin/core/imag-category/src/main.rs index 9629da7f..633729d1 100644 --- a/bin/core/imag-category/src/main.rs +++ b/bin/core/imag-category/src/main.rs @@ -37,6 +37,8 @@ extern crate clap; #[macro_use] extern crate log; +#[macro_use] +extern crate failure; extern crate libimagentrycategory; extern crate libimagerror; @@ -92,7 +94,14 @@ fn main() { fn set(rt: &Runtime) { 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(); + 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))) .into_get_iter(rt.store()) @@ -109,9 +118,16 @@ fn set(rt: &Runtime) { } fn get(rt: &Runtime) { - let sids = rt.ids::().map_err_trace_exit_unwrap(); - let out = rt.stdout(); + 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.into_iter().map(Ok))) .into_get_iter(rt.store()) diff --git a/bin/core/imag-category/src/ui.rs b/bin/core/imag-category/src/ui.rs index fca41bd0..f25aeca0 100644 --- a/bin/core/imag-category/src/ui.rs +++ b/bin/core/imag-category/src/ui.rs @@ -20,11 +20,11 @@ use std::path::PathBuf; use clap::{Arg, ArgMatches, App, SubCommand}; +use failure::Fallible as Result; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { app @@ -103,60 +103,30 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { + fn get_ids(matches: &ArgMatches) -> Result>> { + fn no_ids_error() -> Result>> { + Err(format_err!("Command does not get IDs as input")) + } + + fn get_id_paths(field: &str, subm: &ArgMatches) -> Result>> { + subm.values_of(field) + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() + } + match matches.subcommand() { - ("create-category", _) => { - error!("Command does not get IDs as input"); - ::std::process::exit(1) - }, - - ("delete-category", _) => { - error!("Command does not get IDs as input"); - ::std::process::exit(1) - }, - - ("list-categories", _) => { - error!("Command does not get IDs as input"); - ::std::process::exit(1) - }, - - ("list-category", _) => { - error!("Command does not get IDs as input"); - ::std::process::exit(1) - }, - - ("set", Some(subm)) => { - subm.values_of("set-ids") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - ("get", Some(subm)) => { - subm.values_of("get-ids") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - (other, _) => { - error!("Not a known command: {}", other); - ::std::process::exit(1) - } + ("create-category", _) => no_ids_error(), + ("delete-category", _) => no_ids_error(), + ("list-categories", _) => no_ids_error(), + ("list-category", _) => no_ids_error(), + ("set", Some(subm)) => get_id_paths("set-ids", subm), + ("get", Some(subm)) => get_id_paths("get-ids", subm), + (other, _) => Err(format_err!("Not a known command: {}", other)), } } } diff --git a/bin/core/imag-edit/Cargo.toml b/bin/core/imag-edit/Cargo.toml index 7900ae69..af118f84 100644 --- a/bin/core/imag-edit/Cargo.toml +++ b/bin/core/imag-edit/Cargo.toml @@ -24,6 +24,7 @@ log = "0.4" version = "3" toml = "0.4" toml-query = "0.8" +failure = "0.1" 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/main.rs b/bin/core/imag-edit/src/main.rs index 9f8263bb..ab12be67 100644 --- a/bin/core/imag-edit/src/main.rs +++ b/bin/core/imag-edit/src/main.rs @@ -36,6 +36,7 @@ extern crate clap; #[macro_use] extern crate log; +extern crate failure; extern crate libimagentryedit; extern crate libimagerror; @@ -63,7 +64,14 @@ fn main() { 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(); + 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))) .into_get_iter(rt.store()) diff --git a/bin/core/imag-edit/src/ui.rs b/bin/core/imag-edit/src/ui.rs index 3b788251..81eda1ae 100644 --- a/bin/core/imag-edit/src/ui.rs +++ b/bin/core/imag-edit/src/ui.rs @@ -20,11 +20,11 @@ use std::path::PathBuf; use clap::{Arg, ArgMatches, App}; +use failure::Fallible as Result; use libimagstore::storeid::IntoStoreId; use libimagstore::storeid::StoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { app @@ -54,18 +54,14 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { - matches - .values_of("entry") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() + fn get_ids(matches: &ArgMatches) -> Result>> { + matches.values_of("entry") + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() } } diff --git a/bin/core/imag-gps/src/main.rs b/bin/core/imag-gps/src/main.rs index 0b34605f..ec96721b 100644 --- a/bin/core/imag-gps/src/main.rs +++ b/bin/core/imag-gps/src/main.rs @@ -35,9 +35,8 @@ )] extern crate clap; -#[macro_use] -extern crate log; -extern crate failure; +#[macro_use] extern crate log; +#[macro_use] extern crate failure; extern crate libimagentrygps; #[macro_use] extern crate libimagrt; @@ -52,6 +51,7 @@ use std::str::FromStr; use failure::Error; use failure::err_msg; +use libimagstore::storeid::StoreId; use libimagentrygps::types::*; use libimagentrygps::entry::*; use libimagrt::setup::generate_runtime_setup; @@ -69,8 +69,7 @@ fn main() { "Add GPS coordinates to entries", ui::build_ui); - rt.cli() - .subcommand_name() + rt.cli().subcommand_name() .map(|name| { match name { "add" => add(&rt), @@ -87,6 +86,16 @@ fn main() { }); } +fn rt_get_ids(rt: &Runtime) -> Vec { + rt + .ids::() + .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) +} + fn add(rt: &Runtime) { let c = { let parse = |value: &str| -> (i64, i64, i64) { @@ -124,8 +133,7 @@ fn add(rt: &Runtime) { Coordinates::new(long, lati) }; - rt.ids::() - .map_err_trace_exit_unwrap() + rt_get_ids(&rt) .into_iter() .for_each(|id| { rt.store() @@ -149,8 +157,7 @@ fn remove(rt: &Runtime) { .unwrap() .is_present("print-removed"); // safed by main() - rt.ids::() - .map_err_trace_exit_unwrap() + rt_get_ids(&rt) .into_iter() .for_each(|id| { let removed_value = rt @@ -179,8 +186,8 @@ fn remove(rt: &Runtime) { fn get(rt: &Runtime) { let mut stdout = rt.stdout(); - rt.ids::() - .map_err_trace_exit_unwrap() + + rt_get_ids(&rt) .into_iter() .for_each(|id| { let value = rt diff --git a/bin/core/imag-gps/src/ui.rs b/bin/core/imag-gps/src/ui.rs index 572e1c48..bd7879fe 100644 --- a/bin/core/imag-gps/src/ui.rs +++ b/bin/core/imag-gps/src/ui.rs @@ -20,11 +20,11 @@ use std::path::PathBuf; use clap::{Arg, ArgMatches, App, SubCommand}; +use failure::Fallible as Result; use libimagstore::storeid::IntoStoreId; use libimagstore::storeid::StoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { app @@ -99,54 +99,23 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { + fn get_ids(matches: &ArgMatches) -> Result>> { + fn get_id_paths(field: &str, subm: &ArgMatches) -> Result>> { + subm.values_of(field) + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() + } + match matches.subcommand() { - ("add", Some(subm)) => { - subm.values_of("entry") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - ("remove", Some(subm)) => { - subm.values_of("entry") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - ("get", Some(subm)) => { - subm.values_of("get-ids") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - (other, _) => { - error!("Not a known command: {}", other); - ::std::process::exit(1) - } + ("add", Some(subm)) => get_id_paths("entry", subm), + ("remove", Some(subm)) => get_id_paths("entry", subm), + ("get", Some(subm)) => get_id_paths("get-ids", subm), + (other, _) => Err(format_err!("Not a known command: {}", other)), } } } diff --git a/bin/core/imag-header/src/main.rs b/bin/core/imag-header/src/main.rs index 3e7b01da..10c8805c 100644 --- a/bin/core/imag-header/src/main.rs +++ b/bin/core/imag-header/src/main.rs @@ -79,7 +79,14 @@ fn main() { 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(); + 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.into_iter().map(Ok))) .into_get_iter(rt.store()) diff --git a/bin/core/imag-header/src/ui.rs b/bin/core/imag-header/src/ui.rs index f42030eb..f9d1bcbd 100644 --- a/bin/core/imag-header/src/ui.rs +++ b/bin/core/imag-header/src/ui.rs @@ -20,11 +20,11 @@ use std::path::PathBuf; use clap::{Arg, ArgMatches, App, SubCommand}; +use failure::Fallible as Result; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { app @@ -234,11 +234,14 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { + fn get_ids(matches: &ArgMatches) -> Result>> { matches.values_of("id") - .unwrap() - .map(|s| PathBuf::from(s).into_storeid().map_err_trace_exit_unwrap()) - .collect() + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() } } - diff --git a/bin/core/imag-ids/src/main.rs b/bin/core/imag-ids/src/main.rs index dbe271b2..4284b475 100644 --- a/bin/core/imag-ids/src/main.rs +++ b/bin/core/imag-ids/src/main.rs @@ -41,7 +41,7 @@ extern crate filters; #[macro_use] extern crate is_match; extern crate toml; extern crate toml_query; -extern crate failure; +#[macro_use] extern crate failure; #[cfg(test)] extern crate env_logger; @@ -93,7 +93,13 @@ fn main() { let iterator = if rt.ids_from_stdin() { debug!("Fetching IDs from stdin..."); - let ids = rt.ids::().map_err_trace_exit_unwrap(); + 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 { diff --git a/bin/core/imag-ids/src/ui.rs b/bin/core/imag-ids/src/ui.rs index 1a7e15f2..7cfde684 100644 --- a/bin/core/imag-ids/src/ui.rs +++ b/bin/core/imag-ids/src/ui.rs @@ -18,6 +18,7 @@ // use clap::{Arg, ArgMatches, App, SubCommand}; +use failure::Fallible as Result; use libimagstore::storeid::StoreId; use libimagrt::runtime::IdPathProvider; @@ -54,8 +55,7 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(_matches: &ArgMatches) -> Vec { - error!("imag-ids does not get IDs via CLI, only via stdin if applying a filter!"); - ::std::process::exit(1) + fn get_ids(_matches: &ArgMatches) -> Result>> { + Err(format_err!("imag-ids does not get IDs via CLI, only via stdin if applying a filter!")) } } diff --git a/bin/core/imag-link/src/main.rs b/bin/core/imag-link/src/main.rs index 0255f341..3e12f1c0 100644 --- a/bin/core/imag-link/src/main.rs +++ b/bin/core/imag-link/src/main.rs @@ -209,8 +209,13 @@ fn remove_linking(rt: &Runtime) { }) .unwrap(); - rt.ids::() + rt + .ids::() .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) .into_iter() .for_each(|id| match rt.store().get(id.clone()) { Err(e) => trace_error(&e), @@ -245,19 +250,27 @@ fn remove_linking(rt: &Runtime) { } fn unlink(rt: &Runtime) { - rt.ids::().map_err_trace_exit_unwrap().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(); + 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(); - let _ = rt.report_touched(&id).unwrap_or_exit(); - }); + let _ = rt.report_touched(&id).unwrap_or_exit(); + }); } fn list_linkings(rt: &Runtime) { @@ -271,35 +284,24 @@ fn list_linkings(rt: &Runtime) { let mut tab = ::prettytable::Table::new(); tab.set_titles(row!["#", "Link"]); - rt.ids::().map_err_trace_exit_unwrap().into_iter().for_each(|id| { - match rt.store().get(id.clone()) { - Ok(Some(entry)) => { - for (i, link) in entry.get_internal_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(); - - if let Some(link) = link { - if list_plain { - let _ = writeln!(rt.stdout(), "{: <3}: {}", i, link) - .to_exit_code() - .unwrap_or_exit(); - } else { - tab.add_row(row![i, link]); - } - } - } - - if list_externals { - entry.get_external_links(rt.store()) - .map_err_trace_exit_unwrap() - .enumerate() - .for_each(|(i, link)| { - let link = link - .map_err_trace_exit_unwrap() - .into_string(); + rt + .ids::() + .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) + .into_iter() + .for_each(|id| { + match rt.store().get(id.clone()) { + Ok(Some(entry)) => { + for (i, link) in entry.get_internal_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(); + if let Some(link) = link { if list_plain { let _ = writeln!(rt.stdout(), "{: <3}: {}", i, link) .to_exit_code() @@ -307,18 +309,37 @@ fn list_linkings(rt: &Runtime) { } else { tab.add_row(row![i, link]); } - }) - } + } + } - let _ = rt.report_touched(entry.get_location()).unwrap_or_exit(); + if list_externals { + entry.get_external_links(rt.store()) + .map_err_trace_exit_unwrap() + .enumerate() + .for_each(|(i, link)| { + let link = link + .map_err_trace_exit_unwrap() + .into_string(); - }, - Ok(None) => warn!("Not found: {}", id), - Err(e) => trace_error(&e), - } + if list_plain { + let _ = writeln!(rt.stdout(), "{: <3}: {}", i, link) + .to_exit_code() + .unwrap_or_exit(); + } else { + tab.add_row(row![i, link]); + } + }) + } - let _ = rt.report_touched(&id).unwrap_or_exit(); - }); + let _ = rt.report_touched(entry.get_location()).unwrap_or_exit(); + + }, + Ok(None) => warn!("Not found: {}", id), + Err(e) => trace_error(&e), + } + + let _ = rt.report_touched(&id).unwrap_or_exit(); + }); if !list_plain { let out = rt.stdout(); diff --git a/bin/core/imag-link/src/ui.rs b/bin/core/imag-link/src/ui.rs index 48988add..87973b81 100644 --- a/bin/core/imag-link/src/ui.rs +++ b/bin/core/imag-link/src/ui.rs @@ -20,11 +20,11 @@ use std::path::PathBuf; use clap::{Arg, ArgMatches, App, SubCommand}; +use failure::Fallible as Result; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { app @@ -117,65 +117,25 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { /// It has to be fetched by main() by hand. pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { + fn get_ids(matches: &ArgMatches) -> Result>> { + fn get_id_paths(field: &str, subm: &ArgMatches) -> Result>> { + subm.values_of(field) + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() + } let ids = match matches.subcommand() { - ("remove", Some(subm)) => { - let to = subm.values_of("to") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap(); - Some(to) - }, - - ("unlink", Some(subm)) => { - let ids = subm - .values_of("from") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap(); - - Some(ids) - }, - - ("list", Some(subm)) => { - let ids = subm - .values_of("entries") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap(); - - Some(ids) - }, - + ("remove", Some(subm)) => Some(get_id_paths("to", subm)), + ("unlink", Some(subm)) => Some(get_id_paths("from", subm)), + ("list", Some(subm)) => Some(get_id_paths("entries", subm)), _ => None, }; - ids.unwrap_or_else(|| { - matches.values_of("to") - .unwrap() - .map(|s| PathBuf::from(s).into_storeid().map_err_trace_exit_unwrap()) - .collect() - }) + ids + .unwrap_or_else(|| get_id_paths("to", matches)) } } diff --git a/bin/core/imag-markdown/src/main.rs b/bin/core/imag-markdown/src/main.rs index 0aa087e4..f3c1029b 100644 --- a/bin/core/imag-markdown/src/main.rs +++ b/bin/core/imag-markdown/src/main.rs @@ -65,8 +65,13 @@ fn main() { let out = rt.stdout(); let mut outlock = out.lock(); - let iter = rt.ids::() + let iter = rt + .ids::() .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) .into_iter() .map(Ok) .into_get_iter(rt.store()) diff --git a/bin/core/imag-markdown/src/ui.rs b/bin/core/imag-markdown/src/ui.rs index a2f996c4..ba1ac73a 100644 --- a/bin/core/imag-markdown/src/ui.rs +++ b/bin/core/imag-markdown/src/ui.rs @@ -20,11 +20,11 @@ use std::path::PathBuf; use clap::{Arg, ArgMatches, App}; +use failure::Fallible as Result; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { app @@ -47,18 +47,15 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { + fn get_ids(matches: &ArgMatches) -> Result>> { matches.values_of("entry") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() } } diff --git a/bin/core/imag-ref/src/main.rs b/bin/core/imag-ref/src/main.rs index 197a530a..03a95ccf 100644 --- a/bin/core/imag-ref/src/main.rs +++ b/bin/core/imag-ref/src/main.rs @@ -36,7 +36,7 @@ #[macro_use] extern crate log; extern crate clap; -extern crate failure; +#[macro_use] extern crate failure; extern crate libimagstore; #[macro_use] extern crate libimagrt; @@ -92,12 +92,18 @@ fn main() { fn deref(rt: &Runtime) { let cmd = rt.cli().subcommand_matches("deref").unwrap(); let basepath = cmd.value_of("override-basepath"); - let ids = rt.ids::().map_err_trace_exit_unwrap(); let cfg = get_ref_config(&rt, "imag-ref").map_err_trace_exit_unwrap(); let out = rt.stdout(); let mut outlock = out.lock(); - ids.into_iter() + rt + .ids::<::ui::PathProvider>() + .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) + .into_iter() .for_each(|id| { match rt.store().get(id.clone()).map_err_trace_exit_unwrap() { Some(entry) => { @@ -130,7 +136,6 @@ fn remove(rt: &Runtime) { let cmd = rt.cli().subcommand_matches("remove").unwrap(); let yes = cmd.is_present("yes"); - let ids = rt.ids::().map_err_trace_exit_unwrap(); let mut input = rt.stdin().unwrap_or_else(|| { error!("No input stream. Cannot ask for permission"); @@ -139,7 +144,14 @@ fn remove(rt: &Runtime) { let mut output = rt.stdout(); - ids.into_iter() + rt + .ids::<::ui::PathProvider>() + .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) + .into_iter() .for_each(|id| { match rt.store().get(id.clone()).map_err_trace_exit_unwrap() { Some(mut entry) => { @@ -169,8 +181,13 @@ fn list_dead(rt: &Runtime) { let list_id = cmd.is_present("list-dead-ids"); let mut output = rt.stdout(); - rt.ids::() + rt + .ids::() .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) .into_iter() .for_each(|id| { match rt.store().get(id.clone()).map_err_trace_exit_unwrap() { diff --git a/bin/core/imag-ref/src/ui.rs b/bin/core/imag-ref/src/ui.rs index 21b5bb13..198aa908 100644 --- a/bin/core/imag-ref/src/ui.rs +++ b/bin/core/imag-ref/src/ui.rs @@ -20,11 +20,11 @@ use std::path::PathBuf; use clap::{Arg, App, ArgMatches, SubCommand}; +use failure::Fallible as Result; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { app @@ -133,60 +133,24 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { + fn get_ids(matches: &ArgMatches) -> Result>> { + fn get_id_paths(subm: &ArgMatches) -> Result>> { + subm.values_of("ID") + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() + } + match matches.subcommand() { - ("deref", Some(subm)) => { - subm.values_of("ID") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - ("remove", Some(subm)) => { - subm.values_of("ID") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - ("list-dead", Some(subm)) => { - subm.values_of("ID") - .ok_or_else(|| { - error!("No StoreId found"); - ::std::process::exit(1) - }) - .unwrap() - .into_iter() - .map(PathBuf::from) - .map(|pb| pb.into_storeid()) - .collect::, _>>() - .map_err_trace_exit_unwrap() - }, - - ("create", _) => { - error!("Command does not get IDs as input"); - ::std::process::exit(1) - }, - - - (other, _) => { - error!("Not a known command: {}", other); - ::std::process::exit(1) - } + ("deref", Some(subm)) => get_id_paths(subm), + ("remove", Some(subm)) => get_id_paths(subm), + ("list-dead", Some(subm)) => get_id_paths(subm), + ("create", _) => Err(format_err!("Command does not get IDs as input")), + (other, _) => Err(format_err!("Not a known command: {}", other)), } } } diff --git a/bin/core/imag-tag/Cargo.toml b/bin/core/imag-tag/Cargo.toml index 0bf539f1..6f0c1747 100644 --- a/bin/core/imag-tag/Cargo.toml +++ b/bin/core/imag-tag/Cargo.toml @@ -22,6 +22,7 @@ maintenance = { status = "actively-developed" } [dependencies] log = "0.4.0" toml = "0.4" +failure = "0.1" libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.10.0", path = "../../../lib/core/libimagrt" } @@ -37,7 +38,6 @@ features = ["color", "suggestions", "wrap_help"] [dev-dependencies] toml-query = "0.8" env_logger = "0.5" -failure = "0.1" [dev-dependencies.libimagutil] version = "0.10.0" diff --git a/bin/core/imag-tag/src/main.rs b/bin/core/imag-tag/src/main.rs index 9f1f9bf4..411e01e1 100644 --- a/bin/core/imag-tag/src/main.rs +++ b/bin/core/imag-tag/src/main.rs @@ -38,7 +38,7 @@ extern crate clap; #[macro_use] extern crate log; #[cfg(test)] extern crate toml; -#[cfg(test)] extern crate failure; +extern crate failure; extern crate libimagstore; #[macro_use] extern crate libimagrt; @@ -84,7 +84,14 @@ fn main() { "Direct interface to the store. Use with great care!", build_ui); - let ids = rt.ids::().map_err_trace_exit_unwrap(); + let ids = rt + .ids::() + .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) + .into_iter(); rt.cli() .subcommand_name() diff --git a/bin/core/imag-tag/src/ui.rs b/bin/core/imag-tag/src/ui.rs index 77684d91..232f4bb5 100644 --- a/bin/core/imag-tag/src/ui.rs +++ b/bin/core/imag-tag/src/ui.rs @@ -20,11 +20,11 @@ use std::path::PathBuf; use clap::{Arg, ArgMatches, ArgGroup, App, SubCommand}; +use failure::Fallible as Result; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; use libimagentrytag::tag::is_tag; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { @@ -105,11 +105,15 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { + fn get_ids(matches: &ArgMatches) -> Result>> { matches.values_of("id") - .unwrap() - .map(|s| PathBuf::from(s).into_storeid().map_err_trace_exit_unwrap()) - .collect() + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() } } diff --git a/bin/core/imag-view/src/main.rs b/bin/core/imag-view/src/main.rs index 7ffe61b5..b79ce3f9 100644 --- a/bin/core/imag-view/src/main.rs +++ b/bin/core/imag-view/src/main.rs @@ -82,8 +82,13 @@ fn main() { let view_header = rt.cli().is_present("view-header"); let hide_content = rt.cli().is_present("not-view-content"); - let entries = rt.ids::() + let entries = rt + .ids::<::ui::PathProvider>() .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) .into_iter() .map(Ok) .into_get_iter(rt.store()) diff --git a/bin/core/imag-view/src/ui.rs b/bin/core/imag-view/src/ui.rs index e01d7074..c79fc96d 100644 --- a/bin/core/imag-view/src/ui.rs +++ b/bin/core/imag-view/src/ui.rs @@ -20,11 +20,11 @@ use std::path::PathBuf; use clap::{Arg, ArgMatches, App}; +use failure::Fallible as Result; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagrt::runtime::IdPathProvider; -use libimagerror::trace::MapErrTrace; pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { app @@ -90,10 +90,14 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { + fn get_ids(matches: &ArgMatches) -> Result>> { matches.values_of("id") - .unwrap() - .map(|s| PathBuf::from(s).into_storeid().map_err_trace_exit_unwrap()) - .collect() + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .transpose() } } diff --git a/bin/core/imag/Cargo.toml b/bin/core/imag/Cargo.toml index 2d098127..a5d18657 100644 --- a/bin/core/imag/Cargo.toml +++ b/bin/core/imag/Cargo.toml @@ -20,6 +20,7 @@ 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" log = "0.4.0" [badges] diff --git a/bin/domain/imag-mail/src/main.rs b/bin/domain/imag-mail/src/main.rs index bd6cf9a3..d60861f4 100644 --- a/bin/domain/imag-mail/src/main.rs +++ b/bin/domain/imag-mail/src/main.rs @@ -131,7 +131,7 @@ fn list(rt: &Runtime) { let print_content = scmd.is_present("list-read"); if print_content { - /// TODO: Check whether workaround with "{}" is still necessary when updating "indoc" + // TODO: Check whether workaround with "{}" is still necessary when updating "indoc" warn!("{}", indoc!(r#"You requested to print the content of the mail as well. We use the 'mailparse' crate underneath, but its implementation is nonoptimal. Thus, the content might be printed as empty (no text in the email) @@ -215,8 +215,13 @@ fn list(rt: &Runtime) { } if rt.ids_from_stdin() { - let iter = rt.ids::() + let iter = rt + .ids::() .map_err_trace_exit_unwrap() + .unwrap_or_else(|| { + error!("No ids supplied"); + ::std::process::exit(1); + }) .into_iter() .map(Ok); diff --git a/bin/domain/imag-mail/src/ui.rs b/bin/domain/imag-mail/src/ui.rs index 54a89b63..f06c1e67 100644 --- a/bin/domain/imag-mail/src/ui.rs +++ b/bin/domain/imag-mail/src/ui.rs @@ -18,11 +18,11 @@ // use std::path::PathBuf; +use failure::Fallible as Result; use libimagstore::storeid::StoreId; use libimagrt::runtime::IdPathProvider; use libimagstore::storeid::IntoStoreId; -use libimagerror::trace::MapErrTrace; use clap::{Arg, ArgMatches, App, SubCommand}; @@ -80,15 +80,16 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { pub struct PathProvider; impl IdPathProvider for PathProvider { - fn get_ids(matches: &ArgMatches) -> Vec { - if matches.is_present("list-id") { - matches.values_of("list-id") - .unwrap() - .map(|s| PathBuf::from(s).into_storeid().map_err_trace_exit_unwrap()) - .collect() - } else { - vec![] - } + fn get_ids(matches: &ArgMatches) -> Result>> { + matches.values_of("list-id") + .map(|v| v + .into_iter() + .map(PathBuf::from) + .map(|pb| pb.into_storeid()) + .collect::>>() + ) + .unwrap_or(Ok(Vec::new())) + .map(Some) } } diff --git a/lib/core/libimagrt/src/runtime.rs b/lib/core/libimagrt/src/runtime.rs index 58321938..b1b505b8 100644 --- a/lib/core/libimagrt/src/runtime.rs +++ b/lib/core/libimagrt/src/runtime.rs @@ -398,7 +398,7 @@ impl<'a> Runtime<'a> { self.has_input_pipe } - pub fn ids(&self) -> Result> { + pub fn ids(&self) -> Result>> { use std::io::Read; if self.has_input_pipe { @@ -417,8 +417,9 @@ impl<'a> Runtime<'a> { .map(|id| StoreId::new(id).map_err(Error::from)) .collect() }) + .map(Some) } else { - Ok(T::get_ids(self.cli())) + Ok(T::get_ids(self.cli())?) } } @@ -607,12 +608,11 @@ impl<'a> Runtime<'a> { } } -/// A trait for the path provider functionality +/// A trait for providing ids from clap argument matches /// /// This trait can be implement on a type so that it can provide IDs when given a ArgMatches /// object. -/// -/// It can be used with Runtime::ids() and libimagrt handles "stdin-provides-ids" cases +/// It can be used with Runtime::ids(), and libimagrt handles "stdin-provides-ids" cases /// automatically: /// /// ```ignore @@ -630,13 +630,12 @@ impl<'a> Runtime<'a> { /// /// # Returns /// -/// In case of error, the IdPathProvider::get_ids() function should exit the application -/// with the appropriate error message(s). -/// +/// In case no store ids could be matched, the function should return `Ok(None)` instance. /// On success, the StoreId objects to operate on are returned from the ArgMatches. +/// Otherwise, an appropriate error may be returned. /// pub trait IdPathProvider { - fn get_ids(matches: &ArgMatches) -> Vec; + fn get_ids(matches: &ArgMatches) -> Result>>; } /// Exported for the `imag` command, you probably do not want to use that.