From a71732be491ec47a00ae14c109e60010eba69020 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 4 Sep 2017 16:33:54 +0200 Subject: [PATCH 1/8] Rewrite create() to use positional arg --- bin/core/imag-store/src/create.rs | 63 ++++++++++++------------------- bin/core/imag-store/src/ui.rs | 16 +------- 2 files changed, 27 insertions(+), 52 deletions(-) diff --git a/bin/core/imag-store/src/create.rs b/bin/core/imag-store/src/create.rs index 74ffbec9..5e482ac9 100644 --- a/bin/core/imag-store/src/create.rs +++ b/bin/core/imag-store/src/create.rs @@ -23,9 +23,6 @@ use std::fs::OpenOptions; use std::result::Result as RResult; use std::io::Read; use std::ops::DerefMut; -use std::io::Write; -use std::io::stderr; -use std::process::exit; use clap::ArgMatches; use toml::Value; @@ -44,44 +41,34 @@ use util::build_toml_header; type Result = RResult; pub fn create(rt: &Runtime) { - rt.cli() - .subcommand_matches("create") - .map(|scmd| { - debug!("Found 'create' subcommand..."); + 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").or_else(|| scmd.value_of("id")); - if path.is_none() { - warn!("No ID / Path provided. Exiting now"); - write!(stderr(), "No ID / Path provided. Exiting now").ok(); - exit(1); - } + // unwrap is safe as value is required + let path = scmd.value_of("path").unwrap(); + let path = PathBuf::from(path); + let store = Some(rt.store().path().clone()); + let path = StoreId::new(store, path).unwrap_or_else(|e| trace_error_exit(&e, 1)); - let store_path = rt.store().path().clone(); - let path = match StoreId::new(Some(store_path), PathBuf::from(path.unwrap())) { - Err(e) => trace_error_exit(&e, 1), - Ok(o) => o, - }; - debug!("path = {:?}", path); + debug!("path = {:?}", path); - if scmd.subcommand_matches("entry").is_some() { - debug!("Creating entry from CLI specification"); - create_from_cli_spec(rt, scmd, &path) - .or_else(|_| create_from_source(rt, scmd, &path)) - .or_else(|_| create_with_content_and_header(rt, - &path, - String::new(), - Entry::default_header())) - } else { - debug!("Creating entry"); - create_with_content_and_header(rt, &path, String::new(), - Entry::default_header()) - } - .unwrap_or_else(|e| { - error!("Error building Entry"); - trace_error_exit(&e, 1); - }) - }); + if scmd.subcommand_matches("entry").is_some() { + debug!("Creating entry from CLI specification"); + create_from_cli_spec(rt, scmd, &path) + .or_else(|_| create_from_source(rt, scmd, &path)) + .or_else(|_| create_with_content_and_header(rt, + &path, + String::new(), + Entry::default_header())) + } else { + debug!("Creating entry"); + create_with_content_and_header(rt, &path, String::new(), + Entry::default_header()) + } + .unwrap_or_else(|e| { + error!("Error building Entry"); + trace_error_exit(&e, 1); + }) } fn create_from_cli_spec(rt: &Runtime, matches: &ArgMatches, path: &StoreId) -> Result<()> { diff --git a/bin/core/imag-store/src/ui.rs b/bin/core/imag-store/src/ui.rs index bad56bd1..b8620c46 100644 --- a/bin/core/imag-store/src/ui.rs +++ b/bin/core/imag-store/src/ui.rs @@ -24,29 +24,17 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { .about("Create an entry from the store") .version("0.1") .arg(Arg::with_name("path") - .long("path") - .short("p") + .index(1) .takes_value(true) - .required(false) + .required(true) .help("Create at this store path") .value_name("PATH")) - .arg(Arg::with_name("id") - .long("id") - .short("i") - .takes_value(true) - .required(false) - .help("Same as --path, for consistency") - .value_name("PATH")) .arg(Arg::with_name("from-raw") .long("from-raw") .takes_value(true) .help("Create a new entry by reading this file ('-' for stdin)") .value_name("FILE")) - .group(ArgGroup::with_name("create-destination-group") - .args(&["path", "id"]) - .required(true)) - .subcommand(SubCommand::with_name("entry") .about("Create an entry via commandline") .version("0.1") From 9dde4731f25adf49afe34d9e1b6c470867ff8d0e Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 4 Sep 2017 16:42:31 +0200 Subject: [PATCH 2/8] Rewrite retrieve() to use positional arg --- bin/core/imag-store/src/retrieve.rs | 26 ++++++++++++-------------- bin/core/imag-store/src/ui.rs | 3 +-- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/bin/core/imag-store/src/retrieve.rs b/bin/core/imag-store/src/retrieve.rs index 19c85b16..5c664f93 100644 --- a/bin/core/imag-store/src/retrieve.rs +++ b/bin/core/imag-store/src/retrieve.rs @@ -31,21 +31,19 @@ pub fn retrieve(rt: &Runtime) { rt.cli() .subcommand_matches("retrieve") .map(|scmd| { - scmd.value_of("id") - .map(|id| { - let path = PathBuf::from(id); - let path = try!(StoreId::new(Some(rt.store().path().clone()), path) - .map_err_trace_exit(1)); - debug!("path = {:?}", path); + // unwrap() is safe as arg is required + let id = scmd.value_of("id").unwrap(); + let path = PathBuf::from(id); + let store = Some(rt.store().path().clone()); + let path = try!(StoreId::new(store, path).map_err_trace_exit(1)); + debug!("path = {:?}", path); - rt.store() - // "id" must be present, enforced via clap spec - .retrieve(path) - .map(|e| print_entry(rt, scmd, e)) - .map_dbg_str("No entry") - .map_dbg(|e| format!("{:?}", e)) - .map_err_trace() - }) + rt.store() + .retrieve(path) + .map(|e| print_entry(rt, scmd, e)) + .map_dbg_str("No entry") + .map_dbg(|e| format!("{:?}", e)) + .map_err_trace() }); } diff --git a/bin/core/imag-store/src/ui.rs b/bin/core/imag-store/src/ui.rs index b8620c46..ff014f25 100644 --- a/bin/core/imag-store/src/ui.rs +++ b/bin/core/imag-store/src/ui.rs @@ -69,8 +69,7 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { .about("Retrieve an entry from the store (implicitely creates the entry)") .version("0.1") .arg(Arg::with_name("id") - .long("id") - .short("i") + .index(1) .takes_value(true) .required(true) .help("Retreive by Store Path, where root (/) is the store itself")) From 381223efd929c1d2386efdeaac1eff65a8066c29 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 4 Sep 2017 16:44:23 +0200 Subject: [PATCH 3/8] Rewrite get() to use positional arg --- bin/core/imag-store/src/get.rs | 32 +++++++++++++------------------- bin/core/imag-store/src/ui.rs | 3 +-- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/bin/core/imag-store/src/get.rs b/bin/core/imag-store/src/get.rs index 0c2aa132..051b7029 100644 --- a/bin/core/imag-store/src/get.rs +++ b/bin/core/imag-store/src/get.rs @@ -20,30 +20,24 @@ use std::path::PathBuf; use libimagrt::runtime::Runtime; -use libimagerror::trace::{trace_error, trace_error_exit}; +use libimagerror::trace::trace_error_exit; use libimagstore::storeid::StoreId; use retrieve::print_entry; pub fn get(rt: &Runtime) { - rt.cli() - .subcommand_matches("get") - .map(|scmd| { - scmd.value_of("id") - .map(|id| { - let path = PathBuf::from(id); - let path = match StoreId::new(Some(rt.store().path().clone()), path) { - Err(e) => trace_error_exit(&e, 1), - Ok(p) => p, - }; - debug!("path = {:?}", path); + let scmd = rt.cli().subcommand_matches("get").unwrap(); - match rt.store().get(path) { - Ok(Some(entry)) => print_entry(rt, scmd, entry), - Ok(None) => info!("No entry found"), - Err(e) => trace_error(&e), - } - }) - }); + let id = scmd.value_of("id").unwrap(); // safe by clap + let path = PathBuf::from(id); + let store = Some(rt.store().path().clone()); + let path = StoreId::new(store, path).unwrap_or_else(|e| trace_error_exit(&e, 1)); + debug!("path = {:?}", path); + + let _ = match rt.store().get(path) { + Ok(Some(entry)) => print_entry(rt, scmd, entry), + Ok(None) => info!("No entry found"), + Err(e) => trace_error_exit(&e, 1), + }; } diff --git a/bin/core/imag-store/src/ui.rs b/bin/core/imag-store/src/ui.rs index ff014f25..291518e9 100644 --- a/bin/core/imag-store/src/ui.rs +++ b/bin/core/imag-store/src/ui.rs @@ -111,8 +111,7 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { .about("Get an entry from the store (fails if non-existent)") .version("0.1") .arg(Arg::with_name("id") - .long("id") - .short("i") + .index(1) .takes_value(true) .required(true) .help("Retrieve by Store Path, where root (/) is the store itself") From 9e9c04e5f340670c896d77d3fc32713269c4ae09 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 4 Sep 2017 16:46:32 +0200 Subject: [PATCH 4/8] Rewrite update() to use positional arg --- bin/core/imag-store/src/update.rs | 40 +++++++++++++------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/bin/core/imag-store/src/update.rs b/bin/core/imag-store/src/update.rs index d0f245d5..9fccadaa 100644 --- a/bin/core/imag-store/src/update.rs +++ b/bin/core/imag-store/src/update.rs @@ -27,33 +27,25 @@ use libimagstore::storeid::StoreId; use util::build_toml_header; pub fn update(rt: &Runtime) { - rt.cli() - .subcommand_matches("update") - .map(|scmd| { - scmd.value_of("id") - .map(|id| { - let path = PathBuf::from(id); - let path = match StoreId::new(Some(rt.store().path().clone()), path) { - Err(e) => trace_error_exit(&e, 1), - Ok(p) => p, - }; + let scmd = rt.cli().subcommand_matches("update").unwrap(); + let id = scmd.value_of("id").unwrap(); // Safe by clap + let path = PathBuf::from(id); + let store = Some(rt.store().path().clone()); + let path = StoreId::new(store, path).unwrap_or_else(|e| trace_error_exit(&e, 1)); - rt.store() - .retrieve(path) - .map(|mut locked_e| { - let e = locked_e.deref_mut(); + let _ = rt.store() + .retrieve(path) + .map(|mut locked_e| { + let e = locked_e.deref_mut(); - scmd.value_of("content") - .map(|new_content| { - *e.get_content_mut() = String::from(new_content); - debug!("New content set"); - }); + scmd.value_of("content") + .map(|new_content| { + *e.get_content_mut() = String::from(new_content); + debug!("New content set"); + }); - *e.get_header_mut() = build_toml_header(scmd, e.get_header().clone()); - debug!("New header set"); - }) - }) + *e.get_header_mut() = build_toml_header(scmd, e.get_header().clone()); + debug!("New header set"); }); - } From 7b1ffdfa4ba9d6033e3a5d5bcfe244b05c4ae36c Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 4 Sep 2017 16:51:18 +0200 Subject: [PATCH 5/8] Rewrite delete() to use positional arg --- bin/core/imag-store/src/delete.rs | 29 +++++++++++------------------ bin/core/imag-store/src/ui.rs | 3 +-- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/bin/core/imag-store/src/delete.rs b/bin/core/imag-store/src/delete.rs index 7b9e56ed..4c63bfe2 100644 --- a/bin/core/imag-store/src/delete.rs +++ b/bin/core/imag-store/src/delete.rs @@ -21,29 +21,22 @@ use std::path::PathBuf; use libimagrt::runtime::Runtime; use libimagerror::trace::MapErrTrace; +use libimagerror::trace::trace_error_exit; use libimagstore::storeid::StoreId; -use libimagutil::warn_exit::warn_exit; use libimagutil::warn_result::*; pub fn delete(rt: &Runtime) { - rt.cli() - .subcommand_matches("delete") - .map(|sub| { - sub.value_of("id") - .map(|id| { - let path = PathBuf::from(id); - let path = try!(StoreId::new(Some(rt.store().path().clone()), path) - .map_err_trace_exit(1)); - debug!("Deleting file at {:?}", id); + let scmd = rt.cli().subcommand_matches("delete").unwrap(); + let id = scmd.value_of("id").unwrap(); // safe by clap + let path = PathBuf::from(id); + let store = Some(rt.store().path().clone()); + let path = StoreId::new(store, path).unwrap_or_else(|e| trace_error_exit(&e, 1)); + debug!("Deleting file at {:?}", id); - rt.store() - .delete(path) - .map_warn_err(|e| format!("Error: {:?}", e)) - .map_err_trace_exit(1) - }) - .or_else(|| warn_exit("No ID passed. Will exit now", 1)) - }) - .or_else(|| warn_exit("No subcommand 'delete'. Will exit now", 1)); + let _ = rt.store() + .delete(path) + .map_warn_err(|e| format!("Error: {:?}", e)) + .map_err_trace_exit(1); } #[cfg(test)] diff --git a/bin/core/imag-store/src/ui.rs b/bin/core/imag-store/src/ui.rs index 291518e9..73b259bd 100644 --- a/bin/core/imag-store/src/ui.rs +++ b/bin/core/imag-store/src/ui.rs @@ -179,8 +179,7 @@ pub fn build_ui<'a>(app: App<'a, 'a>) -> App<'a, 'a> { .about("Delete an entry from the store") .version("0.1") .arg(Arg::with_name("id") - .long("id") - .short("i") + .index(1) .takes_value(true) .required(true) .help("Remove Store Entry with this path. Root (/) is the store itself") From 861817a87f0642d62548c3fe93fa9c9c2d37ddee Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 5 Sep 2017 17:38:02 +0200 Subject: [PATCH 6/8] Adapt cli-test for create() In the process fix the binary name. --- bin/core/imag-store/src/create.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/core/imag-store/src/create.rs b/bin/core/imag-store/src/create.rs index 5e482ac9..bb82cb12 100644 --- a/bin/core/imag-store/src/create.rs +++ b/bin/core/imag-store/src/create.rs @@ -175,17 +175,17 @@ mod tests { use toml::Value; make_mock_app! { - app "imag-link"; + app "imag-store"; modulename mock; version "0.4.0"; - with help "imag-link mocking app"; + with help "imag-store mocking app"; } use self::mock::generate_test_runtime; #[test] fn test_create_simple() { let test_name = "test_create_simple"; - let rt = generate_test_runtime(vec!["create", "-p", "test_create_simple"]).unwrap(); + let rt = generate_test_runtime(vec!["create", "test_create_simple"]).unwrap(); create(&rt); From f86d02ecbb3a3d68d7076e6fb38aec83a3e529bf Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 5 Sep 2017 17:38:18 +0200 Subject: [PATCH 7/8] Adapt the cli-test for delete() In the process fix the binary name. --- bin/core/imag-store/src/delete.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/core/imag-store/src/delete.rs b/bin/core/imag-store/src/delete.rs index 4c63bfe2..72da3976 100644 --- a/bin/core/imag-store/src/delete.rs +++ b/bin/core/imag-store/src/delete.rs @@ -47,22 +47,22 @@ mod tests { use std::path::PathBuf; make_mock_app! { - app "imag-link"; + app "imag-store"; modulename mock; version "0.4.0"; - with help "imag-link mocking app"; + with help "imag-store mocking app"; } use self::mock::generate_test_runtime; use self::mock::reset_test_runtime; #[test] - fn test_create_simple() { + fn test_delete_simple() { let test_name = "test_create_simple"; - let rt = generate_test_runtime(vec!["create", "-p", "test_create_simple"]).unwrap(); + let rt = generate_test_runtime(vec!["create", "test_create_simple"]).unwrap(); create(&rt); - let rt = reset_test_runtime(vec!["delete", "--id", "test_create_simple"], rt).unwrap(); + let rt = reset_test_runtime(vec!["delete", "test_create_simple"], rt).unwrap(); delete(&rt); From 30b466b622afca05513570994df40cc28c5fd878 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 5 Sep 2017 22:03:13 +0200 Subject: [PATCH 8/8] Update changelog --- doc/src/09020-changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/src/09020-changelog.md b/doc/src/09020-changelog.md index 13feccdf..7313f9e0 100644 --- a/doc/src/09020-changelog.md +++ b/doc/src/09020-changelog.md @@ -47,6 +47,7 @@ This section contains the changelog from the last release to the next release. * `libimagstore::storeid::StoreId::is_in_collection()` was added * The `libimagentrylink` is now rudimentarily tested * We compile with rustc 1.17, 1.18, .., nightly + * The `imag-store` binary now uses positional arguments in its CLI * Stats ## 0.3.0