Merge pull request #41 from djmitche/refactor-cli

Refactor command-line handling

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"

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"

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<E: std::fmt::Display>(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::<ClapError>() {
+                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)
+    }
+}

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());
+        });
+    }
+}

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| {});
+    }
+}

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| {});
+    }
+}

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<dyn crate::cmd::SubCommand> {
+            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::<Invocation>()
+            .expect("SubComand is not of the expected type");
+        ($check)(si);
+    };
+}

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<Box<dyn SubCommand>> {
+    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<dyn SubCommandInvocation>),
+
+    /// 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<dyn SubCommandInvocation>,
+}
+
+impl CommandInvocation {
+    pub(crate) fn new(subcommand: Box<dyn SubCommandInvocation>) -> 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(),
+        ))
+    }
+}

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| {});
+    }
+}

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<I, T>(iter: I) -> Fallible<CommandInvocation>
+where
+    I: IntoIterator<Item = T>,
+    T: Into<OsString> + 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());
+    }
+}

cli/src/main.rs

@@ -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("Rask")
-        .version("0.1")
-        .author("Dustin J. Mitchell <dustin@v.igoro.us>")
-        .about("Replacement for TaskWarrior")
-        .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!(),
-    };
-}

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<dyn std::error::Error>> {
+    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<dyn std::error::Error>> {
+    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<dyn std::error::Error>> {
+    let mut cmd = Command::cargo_bin("task")?;
+
+    cmd.arg("--no-such-option");
+    cmd.assert()
+        .failure()
+        .stderr(predicate::str::contains("USAGE"));
+
+    Ok(())
+}