From e0b69a62b1245e08380dc099ea24a7d7c9edffe0 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 23 Nov 2020 16:18:24 -0500 Subject: [PATCH 1/2] fix --help metadata --- cli/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index 1ae6b148e..2d71285bc 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -4,10 +4,10 @@ use taskchampion::{taskstorage, Replica, Status}; use uuid::Uuid; fn main() { - let matches = App::new("Rask") + let matches = App::new("TaskChampion") .version("0.1") .author("Dustin J. Mitchell ") - .about("Replacement for TaskWarrior") + .about("Personal task-tracking") .subcommand( SubCommand::with_name("add").about("adds a task").arg( Arg::with_name("description") From fe4183c3cac20065e11a0745f93a2ab4588bac92 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 23 Nov 2020 19:33:04 -0500 Subject: [PATCH 2/2] Refactor command-line handling into modules per subcommands --- Cargo.lock | 114 +++++++++++++++++++++++++++++++++++++++++ cli/Cargo.toml | 5 ++ cli/src/bin/task.rs | 43 ++++++++++++++++ cli/src/cmd/add.rs | 60 ++++++++++++++++++++++ cli/src/cmd/gc.rs | 37 +++++++++++++ cli/src/cmd/list.rs | 39 ++++++++++++++ cli/src/cmd/macros.rs | 45 ++++++++++++++++ cli/src/cmd/mod.rs | 69 +++++++++++++++++++++++++ cli/src/cmd/pending.rs | 49 ++++++++++++++++++ cli/src/lib.rs | 57 +++++++++++++++++++++ cli/src/main.rs | 59 --------------------- cli/tests/cli.rs | 42 +++++++++++++++ 12 files changed, 560 insertions(+), 59 deletions(-) create mode 100644 cli/src/bin/task.rs create mode 100644 cli/src/cmd/add.rs create mode 100644 cli/src/cmd/gc.rs create mode 100644 cli/src/cmd/list.rs create mode 100644 cli/src/cmd/macros.rs create mode 100644 cli/src/cmd/mod.rs create mode 100644 cli/src/cmd/pending.rs create mode 100644 cli/src/lib.rs delete mode 100644 cli/src/main.rs create mode 100644 cli/tests/cli.rs diff --git a/Cargo.lock b/Cargo.lock index e20743939..8471faca1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15,6 +15,15 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ee2a4ec343196209d6594e19543ae87a39f96d5534d7174822a3ad825dd6ed7e" +[[package]] +name = "aho-corasick" +version = "0.7.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7404febffaa47dac81aa44dba71523c9d069b1bdc50a77db41195149e17f68e5" +dependencies = [ + "memchr", +] + [[package]] name = "ansi_term" version = "0.11.0" @@ -24,6 +33,19 @@ dependencies = [ "winapi", ] +[[package]] +name = "assert_cmd" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c88b9ca26f9c16ec830350d309397e74ee9abdfd8eb1f71cb6ecc71a3fc818da" +dependencies = [ + "doc-comment", + "predicates", + "predicates-core", + "predicates-tree", + "wait-timeout", +] + [[package]] name = "atty" version = "0.2.14" @@ -144,6 +166,18 @@ dependencies = [ "bitflags", ] +[[package]] +name = "difference" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "524cbf6897b527295dff137cec09ecf3a05f4fddffd7dfcd1585403449e74198" + +[[package]] +name = "doc-comment" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10" + [[package]] name = "failure" version = "0.1.8" @@ -166,6 +200,15 @@ dependencies = [ "synstructure", ] +[[package]] +name = "float-cmp" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1267f4ac4f343772758f7b1bdcbe767c218bbab93bb432acbf5162bbf85a6c4" +dependencies = [ + "num-traits", +] + [[package]] name = "fnv" version = "1.0.7" @@ -258,6 +301,12 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "memchr" +version = "2.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ee1c47aaa256ecabcaea351eae4a9b01ef39ed810004e298d2511ed284b1525" + [[package]] name = "miniz_oxide" version = "0.4.3" @@ -268,6 +317,12 @@ dependencies = [ "autocfg 1.0.1", ] +[[package]] +name = "normalize-line-endings" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" + [[package]] name = "num-integer" version = "0.1.44" @@ -305,6 +360,35 @@ version = "0.2.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac74c624d6b2d21f425f752262f42188365d7b8ff1aff74c82e45136510a4857" +[[package]] +name = "predicates" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96bfead12e90dccead362d62bb2c90a5f6fc4584963645bc7f71a735e0b0735a" +dependencies = [ + "difference", + "float-cmp", + "normalize-line-endings", + "predicates-core", + "regex", +] + +[[package]] +name = "predicates-core" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06075c3a3e92559ff8929e7a280684489ea27fe44805174c3ebd9328dcb37178" + +[[package]] +name = "predicates-tree" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e63c4859013b38a76eca2414c64911fba30def9e3202ac461a2d22831220124" +dependencies = [ + "predicates-core", + "treeline", +] + [[package]] name = "proc-macro2" version = "1.0.24" @@ -524,6 +608,18 @@ version = "0.1.57" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "41cc0f7e4d5d4544e8861606a285bb08d3e70712ccc7d2b84d7c0ccfaf4b05ce" +[[package]] +name = "regex" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38cf2c13ed4745de91a5eb834e11c00bcc3709e773173b2ce4c56c9fbde04b9c" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", + "thread_local", +] + [[package]] name = "regex-syntax" version = "0.6.21" @@ -667,7 +763,10 @@ dependencies = [ name = "taskchampion-cli" version = "0.1.0" dependencies = [ + "assert_cmd", "clap", + "failure", + "predicates", "taskchampion", "uuid", ] @@ -725,6 +824,15 @@ dependencies = [ "syn", ] +[[package]] +name = "thread_local" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d40c6d1b69745a6ec6fb1ca717914848da4b44ae29d9b3080cbee91d72a69b14" +dependencies = [ + "lazy_static", +] + [[package]] name = "time" version = "0.1.44" @@ -745,6 +853,12 @@ dependencies = [ "serde", ] +[[package]] +name = "treeline" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7f741b240f1a48843f9b8e0444fb55fb2a4ff67293b50a9179dfd5ea67f8d41" + [[package]] name = "unicode-width" version = "0.1.8" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 948b05eb7..75ba8be47 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -8,3 +8,8 @@ edition = "2018" clap = "~2.33.0" uuid = { version = "0.8.1", features = ["serde", "v4"] } taskchampion = { path = "../taskchampion" } +failure = "0.1.8" + +[dev-dependencies] +assert_cmd = "1.0.1" +predicates = "1.0.5" diff --git a/cli/src/bin/task.rs b/cli/src/bin/task.rs new file mode 100644 index 000000000..e90ead9d2 --- /dev/null +++ b/cli/src/bin/task.rs @@ -0,0 +1,43 @@ +use clap::{Error as ClapError, ErrorKind}; +use std::process::exit; +use taskchampion_cli::parse_command_line; + +enum Output { + Stdout, + Stderr, +} +use Output::*; + +fn bail(err: E, output: Output, code: i32) -> ! { + match output { + Stdout => println!("{}", err), + Stderr => eprintln!("{}", err), + } + exit(code) +} + +fn main() { + let command = match parse_command_line(std::env::args_os()) { + Ok(command) => command, + Err(err) => { + match err.downcast::() { + Ok(err) => { + if err.kind == ErrorKind::HelpDisplayed + || err.kind == ErrorKind::VersionDisplayed + { + // --help and --version go to stdout and succeed + bail(err, Stdout, 0) + } else { + // other clap errors exit with failure + bail(err, Stderr, 1) + } + } + Err(err) => bail(err, Stderr, 1), + } + } + }; + + if let Err(err) = command.run() { + bail(err, Stderr, 1) + } +} diff --git a/cli/src/cmd/add.rs b/cli/src/cmd/add.rs new file mode 100644 index 000000000..c34e82453 --- /dev/null +++ b/cli/src/cmd/add.rs @@ -0,0 +1,60 @@ +use clap::{App, Arg, ArgMatches, SubCommand as ClapSubCommand}; +use failure::{format_err, Fallible}; +use taskchampion::Status; +use uuid::Uuid; + +use crate::cmd::{ArgMatchResult, CommandInvocation}; + +#[derive(Debug)] +struct Invocation { + description: String, +} + +define_subcommand! { + fn decorate_app<'a>(&self, app: App<'a, 'a>) -> App<'a, 'a> { + app.subcommand( + ClapSubCommand::with_name("add").about("adds a task").arg( + Arg::with_name("description") + .help("task description") + .required(true), + ), + ) + } + + fn arg_match<'a>(&self, matches: &ArgMatches<'a>) -> ArgMatchResult { + match matches.subcommand() { + ("add", Some(matches)) => { + // TODO: .unwrap() would be safe here as description is required above + let description: String = match matches.value_of("description") { + Some(v) => v.into(), + None => return ArgMatchResult::Err(format_err!("no description provided")), + }; + ArgMatchResult::Ok(Box::new(Invocation { description })) + } + _ => ArgMatchResult::None, + } + } +} + +subcommand_invocation! { + fn run(&self, command: &CommandInvocation) -> Fallible<()> { + let uuid = Uuid::new_v4(); + command + .get_replica() + .new_task(uuid, Status::Pending, self.description.clone()) + .unwrap(); + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn parse_command() { + with_subcommand_invocation!(vec!["task", "add", "foo bar"], |inv: &Invocation| { + assert_eq!(inv.description, "foo bar".to_string()); + }); + } +} diff --git a/cli/src/cmd/gc.rs b/cli/src/cmd/gc.rs new file mode 100644 index 000000000..d8948a715 --- /dev/null +++ b/cli/src/cmd/gc.rs @@ -0,0 +1,37 @@ +use crate::cmd::{ArgMatchResult, CommandInvocation}; +use clap::{App, ArgMatches, SubCommand as ClapSubCommand}; +use failure::Fallible; + +#[derive(Debug)] +struct Invocation {} + +define_subcommand! { + fn decorate_app<'a>(&self, app: App<'a, 'a>) -> App<'a, 'a> { + app.subcommand(ClapSubCommand::with_name("gc").about("run garbage collection")) + } + + fn arg_match<'a>(&self, matches: &ArgMatches<'a>) -> ArgMatchResult { + match matches.subcommand() { + ("gc", _) => ArgMatchResult::Ok(Box::new(Invocation {})), + _ => ArgMatchResult::None, + } + } +} + +subcommand_invocation! { + fn run(&self, command: &CommandInvocation) -> Fallible<()> { + command.get_replica().gc()?; + println!("garbage collected."); + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn parse_command() { + with_subcommand_invocation!(vec!["task", "gc"], |_inv| {}); + } +} diff --git a/cli/src/cmd/list.rs b/cli/src/cmd/list.rs new file mode 100644 index 000000000..fb02e4645 --- /dev/null +++ b/cli/src/cmd/list.rs @@ -0,0 +1,39 @@ +use clap::{App, ArgMatches, SubCommand as ClapSubCommand}; +use failure::Fallible; + +use crate::cmd::{ArgMatchResult, CommandInvocation}; + +#[derive(Debug)] +struct Invocation {} + +define_subcommand! { + fn decorate_app<'a>(&self, app: App<'a, 'a>) -> App<'a, 'a> { + app.subcommand(ClapSubCommand::with_name("list").about("lists tasks")) + } + + fn arg_match<'a>(&self, matches: &ArgMatches<'a>) -> ArgMatchResult { + match matches.subcommand() { + ("list", _) => ArgMatchResult::Ok(Box::new(Invocation {})), + _ => ArgMatchResult::None, + } + } +} + +subcommand_invocation! { + fn run(&self, command: &CommandInvocation) -> Fallible<()> { + for (uuid, task) in command.get_replica().all_tasks().unwrap() { + println!("{} - {:?}", uuid, task); + } + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn parse_command() { + with_subcommand_invocation!(vec!["task", "list"], |_inv| {}); + } +} diff --git a/cli/src/cmd/macros.rs b/cli/src/cmd/macros.rs new file mode 100644 index 000000000..e0fbcd47c --- /dev/null +++ b/cli/src/cmd/macros.rs @@ -0,0 +1,45 @@ +/// Define a Command type implementing SubCommand with the enclosed methods (`decorate_app` and +/// `arg_match`), along with a module-level `cmd` function as the parent module expects. +macro_rules! define_subcommand { + ($($f:item) +) => { + struct Command; + + pub(super) fn cmd() -> Box { + Box::new(Command) + } + + impl crate::cmd::SubCommand for Command { + $($f)+ + } + } +} + +/// Define an Invocation type implementing SubCommandInvocation with the enclosed methods. +macro_rules! subcommand_invocation { + ($($f:item) +) => { + impl crate::cmd::SubCommandInvocation for Invocation { + $($f)+ + + #[cfg(test)] + fn as_any(&self) -> &dyn std::any::Any { + self + } + } + } + +} + +/// Parse the first argument as a command line and convert the result to an Invocation (which must +/// be in scope). If the conversion works, calls the second argument with it. +#[cfg(test)] +macro_rules! with_subcommand_invocation { + ($args:expr, $check:expr) => { + let parsed = crate::parse_command_line($args).unwrap(); + let si = parsed + .subcommand + .as_any() + .downcast_ref::() + .expect("SubComand is not of the expected type"); + ($check)(si); + }; +} diff --git a/cli/src/cmd/mod.rs b/cli/src/cmd/mod.rs new file mode 100644 index 000000000..0ec5ae5fd --- /dev/null +++ b/cli/src/cmd/mod.rs @@ -0,0 +1,69 @@ +use clap::{App, ArgMatches}; +use failure::{Error, Fallible}; +use std::path::Path; +use taskchampion::{taskstorage, Replica}; + +#[macro_use] +mod macros; + +mod add; +mod gc; +mod list; +mod pending; + +/// Get a list of all subcommands in this crate +pub(crate) fn subcommands() -> Vec> { + vec![add::cmd(), gc::cmd(), list::cmd(), pending::cmd()] +} + +/// The result of a [`crate::cmd::SubCommand::arg_match`] call +pub(crate) enum ArgMatchResult { + /// No match + None, + + /// A good match + Ok(Box), + + /// A match, but an issue with the command line + Err(Error), +} + +/// A subcommand represents a defined subcommand, and is typically a singleton. +pub(crate) trait SubCommand { + /// Decorate the given [`clap::App`] appropriately for this subcommand + fn decorate_app<'a>(&self, app: App<'a, 'a>) -> App<'a, 'a>; + + /// If this ArgMatches is for this command, return an appropriate invocation. + fn arg_match<'a>(&self, matches: &ArgMatches<'a>) -> ArgMatchResult; +} + +/// A subcommand invocation is specialized to a subcommand +pub(crate) trait SubCommandInvocation: std::fmt::Debug { + fn run(&self, command: &CommandInvocation) -> Fallible<()>; + + // tests use downcasting, which requires a function to cast to Any + #[cfg(test)] + fn as_any(&self) -> &dyn std::any::Any; +} + +/// A command invocation contains all of the necessary regarding a single invocation of the CLI. +#[derive(Debug)] +pub struct CommandInvocation { + pub(crate) subcommand: Box, +} + +impl CommandInvocation { + pub(crate) fn new(subcommand: Box) -> Self { + Self { subcommand } + } + + pub fn run(self) -> Fallible<()> { + self.subcommand.run(&self) + } + + fn get_replica(&self) -> Replica { + Replica::new(Box::new( + taskstorage::KVStorage::new(Path::new("/tmp/tasks")).unwrap(), + )) + } +} diff --git a/cli/src/cmd/pending.rs b/cli/src/cmd/pending.rs new file mode 100644 index 000000000..e847a4aef --- /dev/null +++ b/cli/src/cmd/pending.rs @@ -0,0 +1,49 @@ +use clap::{App, ArgMatches, SubCommand as ClapSubCommand}; +use failure::Fallible; + +use crate::cmd::{ArgMatchResult, CommandInvocation}; + +#[derive(Debug)] +struct Invocation {} + +define_subcommand! { + fn decorate_app<'a>(&self, app: App<'a, 'a>) -> App<'a, 'a> { + app.subcommand(ClapSubCommand::with_name("pending").about("lists pending tasks")) + } + + fn arg_match<'a>(&self, matches: &ArgMatches<'a>) -> ArgMatchResult { + match matches.subcommand() { + ("pending", _) => ArgMatchResult::Ok(Box::new(Invocation {})), + // default to this command when no subcommand is given + ("", _) => ArgMatchResult::Ok(Box::new(Invocation {})), + _ => ArgMatchResult::None, + } + } +} + +subcommand_invocation! { + fn run(&self, command: &CommandInvocation) -> Fallible<()> { + let working_set = command.get_replica().working_set().unwrap(); + for i in 1..working_set.len() { + if let Some(ref task) = working_set[i] { + println!("{}: {} - {:?}", i, task.get_uuid(), task); + } + } + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn parse_command() { + with_subcommand_invocation!(vec!["task", "pending"], |_inv| {}); + } + + #[test] + fn parse_command_default() { + with_subcommand_invocation!(vec!["task"], |_inv| {}); + } +} diff --git a/cli/src/lib.rs b/cli/src/lib.rs new file mode 100644 index 000000000..af5aed9a7 --- /dev/null +++ b/cli/src/lib.rs @@ -0,0 +1,57 @@ +use clap::{App, AppSettings}; +use failure::Fallible; +use std::ffi::OsString; + +mod cmd; +use cmd::ArgMatchResult; +pub use cmd::CommandInvocation; + +/// Parse the given command line and return an as-yet un-executed CommandInvocation. +pub fn parse_command_line(iter: I) -> Fallible +where + I: IntoIterator, + T: Into + Clone, +{ + let subcommands = cmd::subcommands(); + + let mut app = App::new("TaskChampion") + .version("0.1") + .about("Personal task-tracking") + .setting(AppSettings::ColoredHelp); + + for subcommand in subcommands.iter() { + app = subcommand.decorate_app(app); + } + + let matches = app.get_matches_from_safe(iter)?; + + for subcommand in subcommands.iter() { + match subcommand.arg_match(&matches) { + ArgMatchResult::Ok(invocation) => return Ok(CommandInvocation::new(invocation)), + ArgMatchResult::Err(err) => return Err(err), + ArgMatchResult::None => {} + } + } + + // one of the subcommands also matches the lack of subcommands, so this never + // occurrs. + unreachable!() +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_parse_command_line_success() -> Fallible<()> { + // This just verifies that one of the subcommands works; the subcommands themselves + // are tested in their own unit tests. + parse_command_line(vec!["task", "pending"].iter())?; + Ok(()) + } + + #[test] + fn test_parse_command_line_failure() { + assert!(parse_command_line(vec!["task", "--no-such-arg"].iter()).is_err()); + } +} diff --git a/cli/src/main.rs b/cli/src/main.rs deleted file mode 100644 index 2d71285bc..000000000 --- a/cli/src/main.rs +++ /dev/null @@ -1,59 +0,0 @@ -use clap::{App, Arg, SubCommand}; -use std::path::Path; -use taskchampion::{taskstorage, Replica, Status}; -use uuid::Uuid; - -fn main() { - let matches = App::new("TaskChampion") - .version("0.1") - .author("Dustin J. Mitchell ") - .about("Personal task-tracking") - .subcommand( - SubCommand::with_name("add").about("adds a task").arg( - Arg::with_name("description") - .help("task description") - .required(true), - ), - ) - .subcommand(SubCommand::with_name("list").about("lists tasks")) - .subcommand(SubCommand::with_name("pending").about("lists pending tasks")) - .subcommand(SubCommand::with_name("gc").about("run garbage collection")) - .get_matches(); - - let mut replica = Replica::new(Box::new( - taskstorage::KVStorage::new(Path::new("/tmp/tasks")).unwrap(), - )); - - match matches.subcommand() { - ("add", Some(matches)) => { - let uuid = Uuid::new_v4(); - replica - .new_task( - uuid, - Status::Pending, - matches.value_of("description").unwrap().into(), - ) - .unwrap(); - } - ("list", _) => { - for (uuid, task) in replica.all_tasks().unwrap() { - println!("{} - {:?}", uuid, task); - } - } - ("pending", _) => { - let working_set = replica.working_set().unwrap(); - for i in 1..working_set.len() { - if let Some(ref task) = working_set[i] { - println!("{}: {} - {:?}", i, task.get_uuid(), task); - } - } - } - ("gc", _) => { - replica.gc().unwrap(); - } - ("", None) => { - unreachable!(); - } - _ => unreachable!(), - }; -} diff --git a/cli/tests/cli.rs b/cli/tests/cli.rs new file mode 100644 index 000000000..711adc1a5 --- /dev/null +++ b/cli/tests/cli.rs @@ -0,0 +1,42 @@ +use assert_cmd::prelude::*; +use predicates::prelude::*; +use std::process::Command; + +// This tests that the task binary is running and parsing arguments. The details of subcommands +// are handled with unit tests. + +#[test] +fn help() -> Result<(), Box> { + let mut cmd = Command::cargo_bin("task")?; + + cmd.arg("--help"); + cmd.assert() + .success() + .stdout(predicate::str::contains("Personal task-tracking")); + + Ok(()) +} + +#[test] +fn version() -> Result<(), Box> { + let mut cmd = Command::cargo_bin("task")?; + + cmd.arg("--version"); + cmd.assert() + .success() + .stdout(predicate::str::contains("TaskChampion")); + + Ok(()) +} + +#[test] +fn invalid_option() -> Result<(), Box> { + let mut cmd = Command::cargo_bin("task")?; + + cmd.arg("--no-such-option"); + cmd.assert() + .failure() + .stderr(predicate::str::contains("USAGE")); + + Ok(()) +}