From 09efb330732e8437353b2018667b52958061e6f6 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Wed, 5 May 2021 14:21:01 -0400 Subject: [PATCH 1/4] move Settings to its own module ..and add some tests for it --- cli/src/lib.rs | 1 + cli/src/settings/mod.rs | 226 +-------------------------- cli/src/settings/settings.rs | 287 +++++++++++++++++++++++++++++++++++ 3 files changed, 290 insertions(+), 224 deletions(-) create mode 100644 cli/src/settings/settings.rs diff --git a/cli/src/lib.rs b/cli/src/lib.rs index a846fa9f1..2890e4b9f 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -1,5 +1,6 @@ #![deny(clippy::all)] #![allow(clippy::unnecessary_wraps)] // for Rust 1.50, https://github.com/rust-lang/rust-clippy/pull/6765 +#![allow(clippy::module_inception)] // we use re-exports to shorten stuttering paths like settings::settings::Settings /*! This crate implements the command-line interface to TaskChampion. diff --git a/cli/src/settings/mod.rs b/cli/src/settings/mod.rs index e6c061562..896de38b8 100644 --- a/cli/src/settings/mod.rs +++ b/cli/src/settings/mod.rs @@ -4,230 +4,8 @@ //! startup and not just when those values are used. mod report; +mod settings; mod util; -use crate::argparse::{Condition, Filter}; -use anyhow::{anyhow, Context, Result}; -use std::collections::HashMap; -use std::convert::TryFrom; -use std::env; -use std::fs; -use std::path::PathBuf; -use taskchampion::Status; -use toml::value::Table; -use util::table_with_keys; - pub(crate) use report::{Column, Property, Report, Sort, SortBy}; - -#[derive(Debug)] -pub(crate) struct Settings { - // replica - pub(crate) data_dir: PathBuf, - - // remote sync server - pub(crate) server_client_key: Option, - pub(crate) server_origin: Option, - pub(crate) encryption_secret: Option, - - // local sync server - pub(crate) server_dir: PathBuf, - - // reports - pub(crate) reports: HashMap, -} - -impl Settings { - pub(crate) fn read() -> Result { - if let Some(config_file) = env::var_os("TASKCHAMPION_CONFIG") { - log::debug!("Loading configuration from {:?}", config_file); - env::remove_var("TASKCHAMPION_CONFIG"); - Self::load_from_file(config_file.into(), true) - } else if let Some(mut dir) = dirs_next::config_dir() { - dir.push("taskchampion.toml"); - log::debug!("Loading configuration from {:?} (optional)", dir); - Self::load_from_file(dir, false) - } else { - Ok(Default::default()) - } - } - - fn load_from_file(config_file: PathBuf, required: bool) -> Result { - let mut settings = Self::default(); - - let config_toml = match fs::read_to_string(config_file.clone()) { - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - return if required { - Err(e.into()) - } else { - Ok(settings) - }; - } - Err(e) => return Err(e.into()), - Ok(s) => s, - }; - - let config_toml = config_toml - .parse::() - .with_context(|| format!("error while reading {:?}", config_file))?; - settings - .update_from_toml(&config_toml) - .with_context(|| format!("error while parsing {:?}", config_file))?; - - Ok(settings) - } - - /// Update this object with configuration from the given config file. This is - /// broken out mostly for convenience in error handling - fn update_from_toml(&mut self, config_toml: &toml::Value) -> Result<()> { - let table_keys = [ - "data_dir", - "server_client_key", - "server_origin", - "encryption_secret", - "server_dir", - "reports", - ]; - let table = table_with_keys(&config_toml, &table_keys)?; - - fn get_str_cfg( - table: &Table, - name: &'static str, - setter: F, - ) -> Result<()> { - if let Some(v) = table.get(name) { - setter( - v.as_str() - .ok_or_else(|| anyhow!(".{}: not a string", name))? - .to_owned(), - ); - } - Ok(()) - } - - get_str_cfg(table, "data_dir", |v| { - self.data_dir = v.into(); - })?; - - get_str_cfg(table, "server_client_key", |v| { - self.server_client_key = Some(v); - })?; - - get_str_cfg(table, "server_origin", |v| { - self.server_origin = Some(v); - })?; - - get_str_cfg(table, "encryption_secret", |v| { - self.encryption_secret = Some(v); - })?; - - get_str_cfg(table, "server_dir", |v| { - self.server_dir = v.into(); - })?; - - if let Some(v) = table.get("reports") { - let report_cfgs = v - .as_table() - .ok_or_else(|| anyhow!(".reports: not a table"))?; - for (name, cfg) in report_cfgs { - let report = Report::try_from(cfg).map_err(|e| anyhow!("reports.{}{}", name, e))?; - self.reports.insert(name.clone(), report); - } - } - - Ok(()) - } -} - -impl Default for Settings { - fn default() -> Self { - let data_dir; - let server_dir; - - if let Some(dir) = dirs_next::data_local_dir() { - data_dir = dir.join("taskchampion"); - server_dir = dir.join("taskchampion-sync-server"); - } else { - // fallback - data_dir = PathBuf::from("."); - server_dir = PathBuf::from("."); - } - - // define the default reports - let mut reports = HashMap::new(); - - reports.insert( - "list".to_owned(), - Report { - sort: vec![Sort { - ascending: true, - sort_by: SortBy::Uuid, - }], - columns: vec![ - Column { - label: "id".to_owned(), - property: Property::Id, - }, - Column { - label: "description".to_owned(), - property: Property::Description, - }, - Column { - label: "active".to_owned(), - property: Property::Active, - }, - Column { - label: "tags".to_owned(), - property: Property::Tags, - }, - ], - filter: Default::default(), - }, - ); - - reports.insert( - "next".to_owned(), - Report { - sort: vec![ - Sort { - ascending: true, - sort_by: SortBy::Id, - }, - Sort { - ascending: true, - sort_by: SortBy::Uuid, - }, - ], - columns: vec![ - Column { - label: "id".to_owned(), - property: Property::Id, - }, - Column { - label: "description".to_owned(), - property: Property::Description, - }, - Column { - label: "active".to_owned(), - property: Property::Active, - }, - Column { - label: "tags".to_owned(), - property: Property::Tags, - }, - ], - filter: Filter { - conditions: vec![Condition::Status(Status::Pending)], - }, - }, - ); - - Self { - data_dir, - server_client_key: None, - server_origin: None, - encryption_secret: None, - server_dir, - reports, - } - } -} +pub(crate) use settings::Settings; diff --git a/cli/src/settings/settings.rs b/cli/src/settings/settings.rs new file mode 100644 index 000000000..1c5a2b9dd --- /dev/null +++ b/cli/src/settings/settings.rs @@ -0,0 +1,287 @@ +use super::util::table_with_keys; +use super::{Column, Property, Report, Sort, SortBy}; +use crate::argparse::{Condition, Filter}; +use anyhow::{anyhow, Context, Result}; +use std::collections::HashMap; +use std::convert::TryFrom; +use std::env; +use std::fs; +use std::path::PathBuf; +use taskchampion::Status; +use toml::value::Table; + +#[derive(Debug, PartialEq)] +pub(crate) struct Settings { + // replica + pub(crate) data_dir: PathBuf, + + // remote sync server + pub(crate) server_client_key: Option, + pub(crate) server_origin: Option, + pub(crate) encryption_secret: Option, + + // local sync server + pub(crate) server_dir: PathBuf, + + // reports + pub(crate) reports: HashMap, +} + +impl Settings { + pub(crate) fn read() -> Result { + if let Some(config_file) = env::var_os("TASKCHAMPION_CONFIG") { + log::debug!("Loading configuration from {:?}", config_file); + env::remove_var("TASKCHAMPION_CONFIG"); + Self::load_from_file(config_file.into(), true) + } else if let Some(mut dir) = dirs_next::config_dir() { + dir.push("taskchampion.toml"); + log::debug!("Loading configuration from {:?} (optional)", dir); + Self::load_from_file(dir, false) + } else { + Ok(Default::default()) + } + } + + fn load_from_file(config_file: PathBuf, required: bool) -> Result { + let mut settings = Self::default(); + + let config_toml = match fs::read_to_string(config_file.clone()) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + return if required { + Err(e.into()) + } else { + Ok(settings) + }; + } + Err(e) => return Err(e.into()), + Ok(s) => s, + }; + + let config_toml = config_toml + .parse::() + .with_context(|| format!("error while reading {:?}", config_file))?; + settings + .update_from_toml(&config_toml) + .with_context(|| format!("error while parsing {:?}", config_file))?; + + Ok(settings) + } + + /// Update this object with configuration from the given config file. This is + /// broken out mostly for convenience in error handling + fn update_from_toml(&mut self, config_toml: &toml::Value) -> Result<()> { + let table_keys = [ + "data_dir", + "server_client_key", + "server_origin", + "encryption_secret", + "server_dir", + "reports", + ]; + let table = table_with_keys(&config_toml, &table_keys)?; + + fn get_str_cfg( + table: &Table, + name: &'static str, + setter: F, + ) -> Result<()> { + if let Some(v) = table.get(name) { + setter( + v.as_str() + .ok_or_else(|| anyhow!(".{}: not a string", name))? + .to_owned(), + ); + } + Ok(()) + } + + get_str_cfg(table, "data_dir", |v| { + self.data_dir = v.into(); + })?; + + get_str_cfg(table, "server_client_key", |v| { + self.server_client_key = Some(v); + })?; + + get_str_cfg(table, "server_origin", |v| { + self.server_origin = Some(v); + })?; + + get_str_cfg(table, "encryption_secret", |v| { + self.encryption_secret = Some(v); + })?; + + get_str_cfg(table, "server_dir", |v| { + self.server_dir = v.into(); + })?; + + if let Some(v) = table.get("reports") { + let report_cfgs = v + .as_table() + .ok_or_else(|| anyhow!(".reports: not a table"))?; + for (name, cfg) in report_cfgs { + let report = Report::try_from(cfg).map_err(|e| anyhow!("reports.{}{}", name, e))?; + self.reports.insert(name.clone(), report); + } + } + + Ok(()) + } +} + +impl Default for Settings { + fn default() -> Self { + let data_dir; + let server_dir; + + if let Some(dir) = dirs_next::data_local_dir() { + data_dir = dir.join("taskchampion"); + server_dir = dir.join("taskchampion-sync-server"); + } else { + // fallback + data_dir = PathBuf::from("."); + server_dir = PathBuf::from("."); + } + + // define the default reports + let mut reports = HashMap::new(); + + reports.insert( + "list".to_owned(), + Report { + sort: vec![Sort { + ascending: true, + sort_by: SortBy::Uuid, + }], + columns: vec![ + Column { + label: "id".to_owned(), + property: Property::Id, + }, + Column { + label: "description".to_owned(), + property: Property::Description, + }, + Column { + label: "active".to_owned(), + property: Property::Active, + }, + Column { + label: "tags".to_owned(), + property: Property::Tags, + }, + ], + filter: Default::default(), + }, + ); + + reports.insert( + "next".to_owned(), + Report { + sort: vec![ + Sort { + ascending: true, + sort_by: SortBy::Id, + }, + Sort { + ascending: true, + sort_by: SortBy::Uuid, + }, + ], + columns: vec![ + Column { + label: "id".to_owned(), + property: Property::Id, + }, + Column { + label: "description".to_owned(), + property: Property::Description, + }, + Column { + label: "active".to_owned(), + property: Property::Active, + }, + Column { + label: "tags".to_owned(), + property: Property::Tags, + }, + ], + filter: Filter { + conditions: vec![Condition::Status(Status::Pending)], + }, + }, + ); + + Self { + data_dir, + server_client_key: None, + server_origin: None, + encryption_secret: None, + server_dir, + reports, + } + } +} + +#[cfg(test)] +mod test { + use super::*; + use tempfile::TempDir; + use toml::toml; + + #[test] + fn test_load_from_file_not_required() { + let cfg_dir = TempDir::new().unwrap(); + + let settings = Settings::load_from_file(cfg_dir.path().join("foo.toml"), false).unwrap(); + assert_eq!(settings, Settings::default()); + } + + #[test] + fn test_load_from_file_required() { + let cfg_dir = TempDir::new().unwrap(); + + assert!(Settings::load_from_file(cfg_dir.path().join("foo.toml"), true).is_err()); + } + + #[test] + fn test_load_from_file_exists() { + let cfg_dir = TempDir::new().unwrap(); + fs::write(cfg_dir.path().join("foo.toml"), "data_dir = \"/nowhere\"").unwrap(); + + let settings = Settings::load_from_file(cfg_dir.path().join("foo.toml"), true).unwrap(); + assert_eq!(settings.data_dir, PathBuf::from("/nowhere")); + } + + #[test] + fn test_update_from_toml_top_level_keys() { + let val = toml! { + data_dir = "/data" + server_client_key = "sck" + server_origin = "so" + encryption_secret = "es" + server_dir = "/server" + }; + let mut settings = Settings::default(); + settings.update_from_toml(&val).unwrap(); + + assert_eq!(settings.data_dir, PathBuf::from("/data")); + 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())); + assert_eq!(settings.server_dir, PathBuf::from("/server")); + } + + #[test] + fn test_update_from_toml_report() { + let val = toml! { + [reports.foo] + sort = [ { sort_by = "id" } ] + columns = [ { label = "ID", property = "id" } ] + }; + let mut settings = Settings::default(); + settings.update_from_toml(&val).unwrap(); + + assert!(settings.reports.get("foo").is_some()); + } +} From a778423cbcd59c3d350c15675398fced8ff28920 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Wed, 5 May 2021 14:41:25 -0400 Subject: [PATCH 2/4] store the filename of the loaded config file --- cli/src/settings/settings.rs | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/cli/src/settings/settings.rs b/cli/src/settings/settings.rs index 1c5a2b9dd..935e5e8ab 100644 --- a/cli/src/settings/settings.rs +++ b/cli/src/settings/settings.rs @@ -12,6 +12,9 @@ use toml::value::Table; #[derive(Debug, PartialEq)] pub(crate) struct Settings { + // filename from which this configuration was loaded, if any + pub(crate) filename: Option, + // replica pub(crate) data_dir: PathBuf, @@ -33,15 +36,24 @@ impl Settings { log::debug!("Loading configuration from {:?}", config_file); env::remove_var("TASKCHAMPION_CONFIG"); Self::load_from_file(config_file.into(), true) - } else if let Some(mut dir) = dirs_next::config_dir() { - dir.push("taskchampion.toml"); - log::debug!("Loading configuration from {:?} (optional)", dir); - Self::load_from_file(dir, false) + } else if let Some(filename) = Settings::default_filename() { + log::debug!("Loading configuration from {:?} (optional)", filename); + Self::load_from_file(filename, false) } else { Ok(Default::default()) } } + /// Get the default filename for the configuration, or None if that cannot + /// be determined. + pub(crate) fn default_filename() -> Option { + if let Some(dir) = dirs_next::config_dir() { + Some(dir.join("taskchampion.toml")) + } else { + None + } + } + fn load_from_file(config_file: PathBuf, required: bool) -> Result { let mut settings = Self::default(); @@ -60,6 +72,8 @@ impl Settings { let config_toml = config_toml .parse::() .with_context(|| format!("error while reading {:?}", config_file))?; + + settings.filename = Some(config_file.clone()); settings .update_from_toml(&config_toml) .with_context(|| format!("error while parsing {:?}", config_file))?; @@ -213,6 +227,7 @@ impl Default for Settings { ); Self { + filename: None, data_dir, server_client_key: None, server_origin: None, @@ -247,10 +262,12 @@ mod test { #[test] fn test_load_from_file_exists() { let cfg_dir = TempDir::new().unwrap(); - fs::write(cfg_dir.path().join("foo.toml"), "data_dir = \"/nowhere\"").unwrap(); + let cfg_file = cfg_dir.path().join("foo.toml"); + fs::write(cfg_file.clone(), "data_dir = \"/nowhere\"").unwrap(); - let settings = Settings::load_from_file(cfg_dir.path().join("foo.toml"), true).unwrap(); + let settings = Settings::load_from_file(cfg_file.clone(), true).unwrap(); assert_eq!(settings.data_dir, PathBuf::from("/nowhere")); + assert_eq!(settings.filename, Some(cfg_file)); } #[test] @@ -283,5 +300,6 @@ mod test { settings.update_from_toml(&val).unwrap(); assert!(settings.reports.get("foo").is_some()); + // beyond existence of this report, we can rely on Report's unit tests } } From fd62c8327b223f4f73597d7ce6dc0bed6eb9b894 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Wed, 5 May 2021 14:18:17 -0400 Subject: [PATCH 3/4] Add a `ta config set` subcommand This uses `toml_edit` to edit the config file in-place. For the moment, it only supports top-level arguments, but can be extended to do other things later. --- Cargo.lock | 46 +++++++++++++++++++++ cli/Cargo.toml | 1 + cli/src/argparse/config.rs | 36 +++++++++++++++++ cli/src/argparse/mod.rs | 2 + cli/src/argparse/subcommand.rs | 42 ++++++++++++++++++- cli/src/invocation/cmd/config.rs | 62 ++++++++++++++++++++++++++++ cli/src/invocation/cmd/mod.rs | 1 + cli/src/invocation/mod.rs | 8 ++++ cli/src/settings/settings.rs | 69 +++++++++++++++++++++++++++++--- 9 files changed, 261 insertions(+), 6 deletions(-) create mode 100644 cli/src/argparse/config.rs create mode 100644 cli/src/invocation/cmd/config.rs diff --git a/Cargo.lock b/Cargo.lock index 3e2127449..8a50e2d2a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -324,6 +324,12 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b" +[[package]] +name = "ascii" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eab1c04a571841102f5345a8fc0f6bb3d31c315dec879b5c6e42e40ce7ffa34e" + [[package]] name = "assert_cmd" version = "1.0.3" @@ -573,6 +579,19 @@ dependencies = [ "vec_map", ] +[[package]] +name = "combine" +version = "3.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da3da6baa321ec19e1cc41d31bf599f00c783d0517095cdaf0332e3fe8d20680" +dependencies = [ + "ascii", + "byteorder", + "either", + "memchr", + "unreachable", +] + [[package]] name = "const_fn" version = "0.4.6" @@ -2158,6 +2177,7 @@ dependencies = [ "termcolor", "textwrap 0.13.4", "toml", + "toml_edit", ] [[package]] @@ -2404,6 +2424,17 @@ dependencies = [ "serde", ] +[[package]] +name = "toml_edit" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09391a441b373597cf0888d2b052dcf82c5be4fee05da3636ae30fb57aad8484" +dependencies = [ + "chrono", + "combine", + "linked-hash-map", +] + [[package]] name = "tracing" version = "0.1.25" @@ -2522,6 +2553,15 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f7fe0bb3479651439c9112f72b6c505038574c9fbb575ed1bf3b797fa39dd564" +[[package]] +name = "unreachable" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "382810877fe448991dfc7f0dd6e3ae5d58088fd0ea5e35189655f84e6814fa56" +dependencies = [ + "void", +] + [[package]] name = "untrusted" version = "0.7.1" @@ -2578,6 +2618,12 @@ version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5fecdca9a5291cc2b8dcf7dc02453fee791a280f3743cb0905f8822ae463b3fe" +[[package]] +name = "void" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" + [[package]] name = "wait-timeout" version = "0.2.0" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 8d07f7e21..d66cc63b9 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -15,6 +15,7 @@ textwrap = { version="^0.13.4", features=["terminal_size"] } termcolor = "^1.1.2" atty = "^0.2.14" toml = "^0.5.8" +toml_edit = "^0.2.0" [dependencies.taskchampion] path = "../taskchampion" diff --git a/cli/src/argparse/config.rs b/cli/src/argparse/config.rs new file mode 100644 index 000000000..924164564 --- /dev/null +++ b/cli/src/argparse/config.rs @@ -0,0 +1,36 @@ +use super::args::{any, arg_matching, literal}; +use super::ArgList; +use crate::usage; +use nom::{combinator::*, sequence::*, IResult}; + +#[derive(Debug, PartialEq)] +/// A config operation +pub(crate) enum ConfigOperation { + /// Set a configuration value + Set(String, String), +} + +impl ConfigOperation { + pub(super) fn parse(input: ArgList) -> IResult { + fn set_to_op(input: (&str, &str, &str)) -> Result { + Ok(ConfigOperation::Set(input.1.to_owned(), input.2.to_owned())) + } + map_res( + tuple(( + arg_matching(literal("set")), + arg_matching(any), + arg_matching(any), + )), + set_to_op, + )(input) + } + + pub(super) fn get_usage(u: &mut usage::Usage) { + u.subcommands.push(usage::Subcommand { + name: "config set", + syntax: "config set ", + summary: "Set a configuration value", + description: "Update Taskchampion configuration file to set key = value", + }); + } +} diff --git a/cli/src/argparse/mod.rs b/cli/src/argparse/mod.rs index 3bdac1a8f..88de59046 100644 --- a/cli/src/argparse/mod.rs +++ b/cli/src/argparse/mod.rs @@ -18,12 +18,14 @@ That is, they contain no references, and have no methods to aid in their executi */ mod args; mod command; +mod config; mod filter; mod modification; mod subcommand; pub(crate) use args::TaskId; pub(crate) use command::Command; +pub(crate) use config::ConfigOperation; pub(crate) use filter::{Condition, Filter}; pub(crate) use modification::{DescriptionMod, Modification}; pub(crate) use subcommand::Subcommand; diff --git a/cli/src/argparse/subcommand.rs b/cli/src/argparse/subcommand.rs index 1d7ddc2ba..5609192b7 100644 --- a/cli/src/argparse/subcommand.rs +++ b/cli/src/argparse/subcommand.rs @@ -1,5 +1,5 @@ use super::args::*; -use super::{ArgList, DescriptionMod, Filter, Modification}; +use super::{ArgList, ConfigOperation, DescriptionMod, Filter, Modification}; use crate::usage; use nom::{branch::alt, combinator::*, sequence::*, IResult}; use taskchampion::Status; @@ -25,6 +25,11 @@ pub(crate) enum Subcommand { summary: bool, }, + /// Manipulate configuration + Config { + config_operation: ConfigOperation, + }, + /// Add a new task Add { modification: Modification, @@ -61,6 +66,7 @@ impl Subcommand { all_consuming(alt(( Version::parse, Help::parse, + Config::parse, Add::parse, Modify::parse, Info::parse, @@ -74,6 +80,7 @@ impl Subcommand { pub(super) fn get_usage(u: &mut usage::Usage) { Version::get_usage(u); Help::get_usage(u); + Config::get_usage(u); Add::get_usage(u); Modify::get_usage(u); Info::get_usage(u); @@ -131,6 +138,26 @@ impl Help { fn get_usage(_u: &mut usage::Usage) {} } +struct Config; + +impl Config { + fn parse(input: ArgList) -> IResult { + fn to_subcommand(input: (&str, ConfigOperation)) -> Result { + Ok(Subcommand::Config { + config_operation: input.1, + }) + } + map_res( + tuple((arg_matching(literal("config")), ConfigOperation::parse)), + to_subcommand, + )(input) + } + + fn get_usage(u: &mut usage::Usage) { + ConfigOperation::get_usage(u); + } +} + struct Add; impl Add { @@ -427,6 +454,19 @@ mod test { ); } + #[test] + fn test_config_set() { + assert_eq!( + Subcommand::parse(argv!["config", "set", "x", "y"]).unwrap(), + ( + &EMPTY[..], + Subcommand::Config { + config_operation: ConfigOperation::Set("x".to_owned(), "y".to_owned()) + } + ) + ); + } + #[test] fn test_add_description() { let subcommand = Subcommand::Add { diff --git a/cli/src/invocation/cmd/config.rs b/cli/src/invocation/cmd/config.rs new file mode 100644 index 000000000..0f34defae --- /dev/null +++ b/cli/src/invocation/cmd/config.rs @@ -0,0 +1,62 @@ +use crate::argparse::ConfigOperation; +use crate::settings::Settings; +use termcolor::{ColorSpec, WriteColor}; + +pub(crate) fn execute( + w: &mut W, + config_operation: ConfigOperation, + settings: &Settings, +) -> anyhow::Result<()> { + match config_operation { + ConfigOperation::Set(key, value) => { + let filename = settings.set(&key, &value)?; + write!(w, "Set configuration value ")?; + w.set_color(ColorSpec::new().set_bold(true))?; + write!(w, "{}", &key)?; + w.set_color(ColorSpec::new().set_bold(false))?; + write!(w, " in ")?; + w.set_color(ColorSpec::new().set_bold(true))?; + writeln!(w, "{:?}.", filename)?; + w.set_color(ColorSpec::new().set_bold(false))?; + } + } + Ok(()) +} + +#[cfg(test)] +mod test { + use super::*; + use crate::invocation::test::*; + use std::fs; + use tempfile::TempDir; + + #[test] + fn test_config_set() { + let cfg_dir = TempDir::new().unwrap(); + let cfg_file = cfg_dir.path().join("foo.toml"); + fs::write( + cfg_file.clone(), + "# store data everywhere\ndata_dir = \"/nowhere\"\n", + ) + .unwrap(); + + let settings = Settings::load_from_file(cfg_file.clone(), true).unwrap(); + + let mut w = test_writer(); + + execute( + &mut w, + ConfigOperation::Set("data_dir".to_owned(), "/somewhere".to_owned()), + &settings, + ) + .unwrap(); + assert!(w.into_string().starts_with("Set configuration value ")); + + let updated_toml = fs::read_to_string(cfg_file.clone()).unwrap(); + dbg!(&updated_toml); + assert_eq!( + updated_toml, + "# store data everywhere\ndata_dir = \"/somewhere\"\n" + ); + } +} diff --git a/cli/src/invocation/cmd/mod.rs b/cli/src/invocation/cmd/mod.rs index 18a973ebb..9371f1f2c 100644 --- a/cli/src/invocation/cmd/mod.rs +++ b/cli/src/invocation/cmd/mod.rs @@ -1,6 +1,7 @@ //! Responsible for executing commands as parsed by [`crate::argparse`]. pub(crate) mod add; +pub(crate) mod config; pub(crate) mod gc; pub(crate) mod help; pub(crate) mod info; diff --git a/cli/src/invocation/mod.rs b/cli/src/invocation/mod.rs index 997440b2e..a9723fe9a 100644 --- a/cli/src/invocation/mod.rs +++ b/cli/src/invocation/mod.rs @@ -35,6 +35,10 @@ pub(crate) fn invoke(command: Command, settings: Settings) -> anyhow::Result<()> subcommand: Subcommand::Help { summary }, command_name, } => return cmd::help::execute(&mut w, command_name, summary), + Command { + subcommand: Subcommand::Config { config_operation }, + .. + } => return cmd::config::execute(&mut w, config_operation, &settings), Command { subcommand: Subcommand::Version, .. @@ -90,6 +94,10 @@ pub(crate) fn invoke(command: Command, settings: Settings) -> anyhow::Result<()> subcommand: Subcommand::Help { .. }, .. } => unreachable!(), + Command { + subcommand: Subcommand::Config { .. }, + .. + } => unreachable!(), Command { subcommand: Subcommand::Version, .. diff --git a/cli/src/settings/settings.rs b/cli/src/settings/settings.rs index 935e5e8ab..592874e6a 100644 --- a/cli/src/settings/settings.rs +++ b/cli/src/settings/settings.rs @@ -1,7 +1,7 @@ use super::util::table_with_keys; use super::{Column, Property, Report, Sort, SortBy}; use crate::argparse::{Condition, Filter}; -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use std::collections::HashMap; use std::convert::TryFrom; use std::env; @@ -9,6 +9,7 @@ use std::fs; use std::path::PathBuf; use taskchampion::Status; use toml::value::Table; +use toml_edit::Document; #[derive(Debug, PartialEq)] pub(crate) struct Settings { @@ -46,7 +47,7 @@ impl Settings { /// Get the default filename for the configuration, or None if that cannot /// be determined. - pub(crate) fn default_filename() -> Option { + fn default_filename() -> Option { if let Some(dir) = dirs_next::config_dir() { Some(dir.join("taskchampion.toml")) } else { @@ -54,7 +55,9 @@ impl Settings { } } - fn load_from_file(config_file: PathBuf, required: bool) -> Result { + /// Update this settings object with the contents of the given TOML file. Top-level settings + /// are overwritten, and reports are overwritten by name. + pub(crate) fn load_from_file(config_file: PathBuf, required: bool) -> Result { let mut settings = Self::default(); let config_toml = match fs::read_to_string(config_file.clone()) { @@ -62,6 +65,7 @@ impl Settings { return if required { Err(e.into()) } else { + settings.filename = Some(config_file); Ok(settings) }; } @@ -141,6 +145,40 @@ impl Settings { Ok(()) } + + /// Set a value in the config file, modifying it in place. Returns the filename. + pub(crate) fn set(&self, key: &str, value: &str) -> Result { + let allowed_keys = [ + "data_dir", + "server_client_key", + "server_origin", + "encryption_secret", + "server_dir", + // reports is not allowed, since it is not a string + ]; + if !allowed_keys.contains(&key) { + bail!("No such configuration key {}", key); + } + + let filename = if let Some(ref f) = self.filename { + f.clone() + } else { + Settings::default_filename() + .ok_or_else(|| anyhow!("Could not determine config file name"))? + }; + + let mut document = fs::read_to_string(filename.clone()) + .context("Could not read existing configuration file")? + .parse::() + .context("Could not parse existing configuration file")?; + + document[key] = toml_edit::value(value); + + fs::write(filename.clone(), document.to_string()) + .context("Could not write updated configuration file")?; + + Ok(filename) + } } impl Default for Settings { @@ -247,9 +285,13 @@ mod test { #[test] fn test_load_from_file_not_required() { let cfg_dir = TempDir::new().unwrap(); + let cfg_file = cfg_dir.path().join("foo.toml"); - let settings = Settings::load_from_file(cfg_dir.path().join("foo.toml"), false).unwrap(); - assert_eq!(settings, Settings::default()); + let settings = Settings::load_from_file(cfg_file.clone(), false).unwrap(); + + let mut expected = Settings::default(); + expected.filename = Some(cfg_file.clone()); + assert_eq!(settings, expected); } #[test] @@ -302,4 +344,21 @@ mod test { assert!(settings.reports.get("foo").is_some()); // beyond existence of this report, we can rely on Report's unit tests } + + #[test] + fn test_set_valid_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())); + settings.set("data_dir", "/data").unwrap(); + + // load the file again and see the change + 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)); + } } From 3bb198425cb3b456538154f78350616ee6ff2a60 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Wed, 5 May 2021 16:25:05 -0400 Subject: [PATCH 4/4] Use `ta config set` in documentation --- docs/src/config-file.md | 10 +++++++++- docs/src/task-sync.md | 28 +++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/docs/src/config-file.md b/docs/src/config-file.md index 90a892805..6968e6e51 100644 --- a/docs/src/config-file.md +++ b/docs/src/config-file.md @@ -40,7 +40,15 @@ If using a remote server: * `server_client_key` - Client key to identify this replica to the sync server (a UUID) If not set, then sync is done to a local server. -# Reports +## Reports * `reports` - a mapping of each report's name to its definition. See [Reports](./reports.md) for details. + +## Editing + +As a shortcut, the simple, top-level configuration values can be edited from the command line: + +```shell +ta config set data_dir /home/myuser/.taskchampion +``` diff --git a/docs/src/task-sync.md b/docs/src/task-sync.md index a4252c6ac..82200ae48 100644 --- a/docs/src/task-sync.md +++ b/docs/src/task-sync.md @@ -11,13 +11,39 @@ Synchronization is quick, especially if no changes have occurred. Each replica expects to be synchronized frequently, even if no server is involved. Without periodic syncs, the storage space used for the task database will grow quickly, and performance will suffer. +## Local Sync + By default, TaskChampion syncs to a "local server", as specified by the `server_dir` configuration parameter. +This defaults to `taskchampion-sync-server` in your [data directory](https://docs.rs/dirs-next/2.0.0/dirs_next/fn.data_dir.html), but can be customized in the configuration file. + +## Remote Sync + +For remote synchronization, you will need a few pieces of information. +From the server operator, you will need an origin and a client key. +Configure these with + +```shell +ta config set server_origin "" +ta config set server_client_key "" +``` + +You will need to generate your own encryption secret. +This is used to encrypt your task history, so treat it as a password. +The following will use the `openssl` utility to generate a suitable value: + +```shell +ta config set encryption_secret $(openssl rand -hex 35) +``` + Every replica sharing a task history should have precisely the same configuration for `server_origin`, `server_client_key`, and `encryption_secret`. +### Adding a New Replica + Synchronizing a new replica to an existing task history is easy: begin with an empty replica, configured for the remote server, and run `ta sync`. The replica will download the entire task history. +### Upgrading a Locally-Sync'd Replica + It is possible to switch a single replica to a remote server by simply configuring for the remote server and running `ta sync`. The replica will upload the entire task history to the server. Once this is complete, additional replicas can be configured with the same settings in order to share the task history. -