From 0de4fc1dee42ddc8ab700a7107e85d4f2d887c28 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 7 Jun 2021 14:57:57 -0400 Subject: [PATCH] Add confirmation prompts for modifications of lots of tasks --- Cargo.lock | 34 +++++++++++++++++ cli/Cargo.toml | 1 + cli/src/invocation/cmd/modify.rs | 50 ++++++++++++++++++++++-- cli/src/invocation/mod.rs | 16 ++------ cli/src/invocation/util.rs | 22 +++++++++++ cli/src/settings/settings.rs | 65 ++++++++++++++++++++++++++++---- docs/src/config-file.md | 6 +++ 7 files changed, 170 insertions(+), 24 deletions(-) create mode 100644 cli/src/invocation/util.rs diff --git a/Cargo.lock b/Cargo.lock index c28fa2e1e..316c167e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -665,6 +665,21 @@ dependencies = [ "unreachable", ] +[[package]] +name = "console" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3993e6445baa160675931ec041a5e03ca84b9c6e32a056150d3aa2bdda0a1f45" +dependencies = [ + "encode_unicode", + "lazy_static", + "libc", + "regex", + "terminal_size", + "unicode-width", + "winapi 0.3.9", +] + [[package]] name = "const_fn" version = "0.4.6" @@ -760,6 +775,18 @@ dependencies = [ "syn", ] +[[package]] +name = "dialoguer" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c9dd058f8b65922819fabb4a41e7d1964e56344042c26efbccd465202c23fa0c" +dependencies = [ + "console", + "lazy_static", + "tempfile", + "zeroize", +] + [[package]] name = "difference" version = "2.0.0" @@ -2972,6 +2999,7 @@ dependencies = [ "atty", "built", "chrono", + "dialoguer", "dirs-next", "env_logger 0.8.3", "iso8601-duration", @@ -3784,3 +3812,9 @@ dependencies = [ "markup5ever", "time 0.1.43", ] + +[[package]] +name = "zeroize" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4756f7db3f7b5574938c3eb1c117038b8e07f95ee6718c0efad4ac21508f1efd" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index bcc34aeda..feada5597 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -25,6 +25,7 @@ toml_edit = "^0.2.0" chrono = "0.4" lazy_static = "1" iso8601-duration = "0.1" +dialoguer = "0.8" # only needed for usage-docs # if the mdbook version changes, change it in .github/workflows/publish-docs.yml and .github/workflows/checks.yml as well diff --git a/cli/src/invocation/cmd/modify.rs b/cli/src/invocation/cmd/modify.rs index c3fee9a40..0fa803441 100644 --- a/cli/src/invocation/cmd/modify.rs +++ b/cli/src/invocation/cmd/modify.rs @@ -1,15 +1,58 @@ use crate::argparse::{Filter, Modification}; -use crate::invocation::{apply_modification, filtered_tasks, summarize_task}; +use crate::invocation::util::{confirm, summarize_task}; +use crate::invocation::{apply_modification, filtered_tasks}; +use crate::settings::Settings; use taskchampion::Replica; use termcolor::WriteColor; +/// confirm modification of more than `modificationt_count_prompt` tasks, defaulting to 3 +fn check_modification( + w: &mut W, + settings: &Settings, + affected_tasks: usize, +) -> Result { + let setting = settings.modification_count_prompt.unwrap_or(3); + if setting == 0 || affected_tasks <= setting as usize { + return Ok(true); + } + + let prompt = format!("Operation will modify {} tasks; continue?", affected_tasks,); + if confirm(&prompt)? { + return Ok(true); + } + + writeln!(w, "Cancelled")?; + + // only show this help if the setting is not set + if settings.modification_count_prompt.is_none() { + writeln!( + w, + "Set the `modification_count_prompt` setting to avoid this prompt:" + )?; + writeln!( + w, + " ta config set modification_count_prompt {}", + affected_tasks + 1 + )?; + writeln!(w, "Set it to 0 to disable the prompt entirely")?; + } + Ok(false) +} + pub(crate) fn execute( w: &mut W, replica: &mut Replica, + settings: &Settings, filter: Filter, modification: Modification, ) -> Result<(), crate::Error> { - for task in filtered_tasks(replica, &filter)? { + let tasks = filtered_tasks(replica, &filter)?; + + if !check_modification(w, settings, tasks.size_hint().0)? { + return Ok(()); + } + + for task in tasks { let mut task = task.into_mut(replica); apply_modification(&mut task, &modification)?; @@ -34,6 +77,7 @@ mod test { fn test_modify() { let mut w = test_writer(); let mut replica = test_replica(); + let settings = Settings::default(); let task = replica .new_task(Status::Pending, s!("old description")) @@ -46,7 +90,7 @@ mod test { description: DescriptionMod::Set(s!("new description")), ..Default::default() }; - execute(&mut w, &mut replica, filter, modification).unwrap(); + execute(&mut w, &mut replica, &settings, filter, modification).unwrap(); // check that the task appeared.. let task = replica.get_task(task.get_uuid()).unwrap().unwrap(); diff --git a/cli/src/invocation/mod.rs b/cli/src/invocation/mod.rs index 80c49db75..b2335a345 100644 --- a/cli/src/invocation/mod.rs +++ b/cli/src/invocation/mod.rs @@ -2,13 +2,14 @@ use crate::argparse::{Command, Subcommand}; use crate::settings::Settings; -use taskchampion::{Replica, Server, ServerConfig, StorageConfig, Task, Uuid}; +use taskchampion::{Replica, Server, ServerConfig, StorageConfig, Uuid}; use termcolor::{ColorChoice, StandardStream}; mod cmd; mod filter; mod modify; mod report; +mod util; #[cfg(test)] mod test; @@ -60,7 +61,7 @@ pub(crate) fn invoke(command: Command, settings: Settings) -> Result<(), crate:: modification, }, .. - } => return cmd::modify::execute(&mut w, &mut replica, filter, modification), + } => return cmd::modify::execute(&mut w, &mut replica, &settings, filter, modification), Command { subcommand: @@ -149,14 +150,3 @@ fn get_writer() -> StandardStream { ColorChoice::Never }) } - -/// Summarize a task in a single line -fn summarize_task(replica: &mut Replica, task: &Task) -> anyhow::Result { - let ws = replica.working_set()?; - let uuid = task.get_uuid(); - if let Some(id) = ws.by_uuid(uuid) { - Ok(format!("{} - {}", id, task.get_description())) - } else { - Ok(format!("{} - {}", uuid, task.get_description())) - } -} diff --git a/cli/src/invocation/util.rs b/cli/src/invocation/util.rs new file mode 100644 index 000000000..12f4535e8 --- /dev/null +++ b/cli/src/invocation/util.rs @@ -0,0 +1,22 @@ +use dialoguer::Confirm; +use taskchampion::{Replica, Task}; + +/// Print the prompt and ask the user to answer yes or no. If input is not from a terminal, the +/// answer is assumed to be true. +pub(super) fn confirm>(prompt: S) -> anyhow::Result { + if !atty::is(atty::Stream::Stdin) { + return Ok(true); + } + Ok(Confirm::new().with_prompt(prompt).interact()?) +} + +/// Summarize a task in a single line +pub(super) fn summarize_task(replica: &mut Replica, task: &Task) -> anyhow::Result { + let ws = replica.working_set()?; + let uuid = task.get_uuid(); + if let Some(id) = ws.by_uuid(uuid) { + Ok(format!("{} - {}", id, task.get_description())) + } else { + Ok(format!("{} - {}", uuid, task.get_description())) + } +} diff --git a/cli/src/settings/settings.rs b/cli/src/settings/settings.rs index 955520299..95d0e4d2e 100644 --- a/cli/src/settings/settings.rs +++ b/cli/src/settings/settings.rs @@ -13,21 +13,25 @@ use toml_edit::Document; #[derive(Debug, PartialEq)] pub(crate) struct Settings { - // filename from which this configuration was loaded, if any + /// filename from which this configuration was loaded, if any pub(crate) filename: Option, - // replica + /// Maximum number of tasks to modify without a confirmation prompt; `Some(0)` means to never + /// prompt, and `None` means to use the default value. + pub(crate) modification_count_prompt: Option, + + /// replica pub(crate) data_dir: PathBuf, - // remote sync server + /// remote sync server pub(crate) server_client_key: Option, pub(crate) server_origin: Option, pub(crate) encryption_secret: Option, - // local sync server + /// local sync server pub(crate) server_dir: PathBuf, - // reports + /// reports pub(crate) reports: HashMap, } @@ -86,6 +90,7 @@ impl Settings { fn update_from_toml(&mut self, config_toml: &toml::Value) -> Result<()> { let table_keys = [ "data_dir", + "modification_count_prompt", "server_client_key", "server_origin", "encryption_secret", @@ -109,10 +114,24 @@ impl Settings { Ok(()) } + fn get_i64_cfg(table: &Table, name: &'static str, setter: F) -> Result<()> { + if let Some(v) = table.get(name) { + setter( + v.as_integer() + .ok_or_else(|| anyhow!(".{}: not a number", name))?, + ); + } + Ok(()) + } + get_str_cfg(table, "data_dir", |v| { self.data_dir = v.into(); })?; + get_i64_cfg(table, "modification_count_prompt", |v| { + self.modification_count_prompt = Some(v); + })?; + get_str_cfg(table, "server_client_key", |v| { self.server_client_key = Some(v); })?; @@ -142,10 +161,12 @@ impl Settings { Ok(()) } - /// Set a value in the config file, modifying it in place. Returns the filename. + /// Set a value in the config file, modifying it in place. Returns the filename. The value is + /// interpreted as the appropriate type for the configuration setting. pub(crate) fn set(&self, key: &str, value: &str) -> Result { let allowed_keys = [ "data_dir", + "modification_count_prompt", "server_client_key", "server_origin", "encryption_secret", @@ -168,7 +189,17 @@ impl Settings { .parse::() .context("Could not parse existing configuration file")?; - document[key] = toml_edit::value(value); + // set the value as the correct type + match key { + // integers + "modification_count_prompt" => { + let value: i64 = value.parse()?; + document[key] = toml_edit::value(value); + } + + // most keys are strings + _ => document[key] = toml_edit::value(value), + } fs::write(filename.clone(), document.to_string()) .context("Could not write updated configuration file")?; @@ -267,6 +298,7 @@ impl Default for Settings { Self { filename: None, data_dir, + modification_count_prompt: None, server_client_key: None, server_origin: None, encryption_secret: None, @@ -316,6 +348,7 @@ mod test { fn test_update_from_toml_top_level_keys() { let val = toml! { data_dir = "/data" + modification_count_prompt = 42 server_client_key = "sck" server_origin = "so" encryption_secret = "es" @@ -325,6 +358,7 @@ mod test { settings.update_from_toml(&val).unwrap(); assert_eq!(settings.data_dir, PathBuf::from("/data")); + assert_eq!(settings.modification_count_prompt, Some(42)); assert_eq!(settings.server_client_key, Some("sck".to_owned())); assert_eq!(settings.server_origin, Some("so".to_owned())); assert_eq!(settings.encryption_secret, Some("es".to_owned())); @@ -354,11 +388,26 @@ mod test { let settings = Settings::load_from_file(cfg_file.clone(), true).unwrap(); assert_eq!(settings.filename, Some(cfg_file.clone())); settings.set("data_dir", "/data").unwrap(); + settings.set("modification_count_prompt", "42").unwrap(); - // load the file again and see the change + // load the file again and see the changes let settings = Settings::load_from_file(cfg_file.clone(), true).unwrap(); assert_eq!(settings.data_dir, PathBuf::from("/data")); assert_eq!(settings.server_dir, PathBuf::from("/srv")); assert_eq!(settings.filename, Some(cfg_file)); + assert_eq!(settings.modification_count_prompt, Some(42)); + } + + #[test] + fn test_set_invalid_key() { + let cfg_dir = TempDir::new().unwrap(); + let cfg_file = cfg_dir.path().join("foo.toml"); + fs::write(cfg_file.clone(), "server_dir = \"/srv\"").unwrap(); + + let settings = Settings::load_from_file(cfg_file.clone(), true).unwrap(); + assert_eq!(settings.filename, Some(cfg_file.clone())); + assert!(settings + .set("modification_count_prompt", "a string?") + .is_err()); } } diff --git a/docs/src/config-file.md b/docs/src/config-file.md index 6968e6e51..03639546f 100644 --- a/docs/src/config-file.md +++ b/docs/src/config-file.md @@ -20,6 +20,12 @@ data_dir = "/home/myuser/.tasks" * `data_dir` - path to a directory containing the replica's task data (which will be created if necessary). Default: `taskchampion` in the local data directory. +## Command-Line Preferences + +* `modification_count_prompt` - when a modification will affect more than this many tasks, the `ta` command will prompt for confirmation. + A value of `0` will disable the prompts entirely. + Default: 3. + ## Sync Server If using a local server: