From 46c3b312083b7fb194b04a344a88ce7d114016a3 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Wed, 30 Dec 2020 20:40:46 +0000 Subject: [PATCH] Parse reports from config, with defaults --- cli/src/argparse/filter.rs | 89 +++--- cli/src/invocation/cmd/report.rs | 8 +- cli/src/invocation/mod.rs | 2 +- cli/src/invocation/report.rs | 63 +--- cli/src/report.rs | 510 ++++++++++++++++++++++++++++++- cli/src/settings.rs | 44 ++- 6 files changed, 624 insertions(+), 92 deletions(-) diff --git a/cli/src/argparse/filter.rs b/cli/src/argparse/filter.rs index 6a8033eb3..812917845 100644 --- a/cli/src/argparse/filter.rs +++ b/cli/src/argparse/filter.rs @@ -1,6 +1,7 @@ use super::args::{arg_matching, id_list, minus_tag, plus_tag, status_colon, TaskId}; use super::ArgList; use crate::usage; +use failure::{bail, Fallible}; use nom::{branch::alt, combinator::*, multi::fold_many0, IResult}; use taskchampion::Status; @@ -32,15 +33,61 @@ pub(crate) enum Condition { IdList(Vec), } +impl Condition { + fn parse(input: ArgList) -> IResult { + alt(( + Self::parse_id_list, + Self::parse_plus_tag, + Self::parse_minus_tag, + Self::parse_status, + ))(input) + } + + /// Parse a single condition string + pub(crate) fn parse_str(input: &str) -> Fallible { + let input = &[input]; + Ok(match Condition::parse(input) { + Ok((&[], cond)) => cond, + Ok(_) => unreachable!(), // input only has one element + Err(nom::Err::Incomplete(_)) => unreachable!(), + Err(nom::Err::Error(e)) => bail!("invalid filter condition: {:?}", e), + Err(nom::Err::Failure(e)) => bail!("invalid filter condition: {:?}", e), + }) + } + + fn parse_id_list(input: ArgList) -> IResult { + fn to_condition(input: Vec) -> Result { + Ok(Condition::IdList(input)) + } + map_res(arg_matching(id_list), to_condition)(input) + } + + fn parse_plus_tag(input: ArgList) -> IResult { + fn to_condition(input: &str) -> Result { + Ok(Condition::HasTag(input.to_owned())) + } + map_res(arg_matching(plus_tag), to_condition)(input) + } + + fn parse_minus_tag(input: ArgList) -> IResult { + fn to_condition(input: &str) -> Result { + Ok(Condition::NoTag(input.to_owned())) + } + map_res(arg_matching(minus_tag), to_condition)(input) + } + + fn parse_status(input: ArgList) -> IResult { + fn to_condition(input: Status) -> Result { + Ok(Condition::Status(input)) + } + map_res(arg_matching(status_colon), to_condition)(input) + } +} + impl Filter { pub(super) fn parse(input: ArgList) -> IResult { fold_many0( - alt(( - Self::parse_id_list, - Self::parse_plus_tag, - Self::parse_minus_tag, - Self::parse_status, - )), + Condition::parse, Filter { ..Default::default() }, @@ -80,36 +127,6 @@ impl Filter { self } - // parsers - - fn parse_id_list(input: ArgList) -> IResult { - fn to_condition(input: Vec) -> Result { - Ok(Condition::IdList(input)) - } - map_res(arg_matching(id_list), to_condition)(input) - } - - fn parse_plus_tag(input: ArgList) -> IResult { - fn to_condition(input: &str) -> Result { - Ok(Condition::HasTag(input.to_owned())) - } - map_res(arg_matching(plus_tag), to_condition)(input) - } - - fn parse_minus_tag(input: ArgList) -> IResult { - fn to_condition(input: &str) -> Result { - Ok(Condition::NoTag(input.to_owned())) - } - map_res(arg_matching(minus_tag), to_condition)(input) - } - - fn parse_status(input: ArgList) -> IResult { - fn to_condition(input: Status) -> Result { - Ok(Condition::Status(input)) - } - map_res(arg_matching(status_colon), to_condition)(input) - } - // usage pub(super) fn get_usage(u: &mut usage::Usage) { diff --git a/cli/src/invocation/cmd/report.rs b/cli/src/invocation/cmd/report.rs index 38a72b3e7..9076c7b23 100644 --- a/cli/src/invocation/cmd/report.rs +++ b/cli/src/invocation/cmd/report.rs @@ -1,5 +1,6 @@ use crate::argparse::Filter; use crate::invocation::display_report; +use config::Config; use failure::Fallible; use taskchampion::Replica; use termcolor::WriteColor; @@ -7,10 +8,11 @@ use termcolor::WriteColor; pub(crate) fn execute( w: &mut W, replica: &mut Replica, + settings: &Config, report_name: String, filter: Filter, ) -> Fallible<()> { - display_report(w, replica, report_name, filter) + display_report(w, replica, settings, report_name, filter) } #[cfg(test)] @@ -29,11 +31,13 @@ mod test { // The function being tested is only one line long, so this is sort of an integration test // for display_report. + let settings = crate::settings::default_settings().unwrap(); let report_name = "next".to_owned(); let filter = Filter { ..Default::default() }; - execute(&mut w, &mut replica, report_name, filter).unwrap(); + + execute(&mut w, &mut replica, &settings, report_name, filter).unwrap(); assert!(w.into_string().contains("my task")); } } diff --git a/cli/src/invocation/mod.rs b/cli/src/invocation/mod.rs index 6cbc0df03..3d5a953dc 100644 --- a/cli/src/invocation/mod.rs +++ b/cli/src/invocation/mod.rs @@ -66,7 +66,7 @@ pub(crate) fn invoke(command: Command, settings: Config) -> Fallible<()> { filter, }, .. - } => return cmd::report::execute(&mut w, &mut replica, report_name, filter), + } => return cmd::report::execute(&mut w, &mut replica, &settings, report_name, filter), Command { subcommand: Subcommand::Info { filter, debug }, diff --git a/cli/src/invocation/report.rs b/cli/src/invocation/report.rs index 82cc67bba..a6e35e72f 100644 --- a/cli/src/invocation/report.rs +++ b/cli/src/invocation/report.rs @@ -1,11 +1,12 @@ -use crate::argparse::{Condition, Filter}; +use crate::argparse::Filter; use crate::invocation::filtered_tasks; -use crate::report::{Column, Property, Report, Sort, SortBy}; +use crate::report::{Column, Property, Report, SortBy}; use crate::table; -use failure::{bail, Fallible}; +use config::Config; +use failure::{format_err, Fallible}; use prettytable::{Row, Table}; use std::cmp::Ordering; -use taskchampion::{Replica, Status, Task, Uuid}; +use taskchampion::{Replica, Task, Uuid}; use termcolor::WriteColor; // pending #123, this is a non-fallible way of looking up a task's working set index @@ -102,61 +103,23 @@ fn task_column(task: &Task, column: &Column, working_set: &WorkingSet) -> String } } -fn get_report(report_name: String, filter: Filter) -> Fallible { - let 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, - }, - ]; - let sort = vec![Sort { - ascending: false, - sort_by: SortBy::Uuid, - }]; - let mut report = match report_name.as_ref() { - "list" => Report { - columns, - sort, - filter: Default::default(), - }, - "next" => Report { - columns, - sort, - filter: Filter { - conditions: vec![Condition::Status(Status::Pending)], - }, - }, - _ => bail!("Unknown report {:?}", report_name), - }; - - // intersect the report's filter with the user-supplied filter - report.filter = report.filter.intersect(filter); - - Ok(report) -} - pub(super) fn display_report( w: &mut W, replica: &mut Replica, + settings: &Config, report_name: String, filter: Filter, ) -> Fallible<()> { let mut t = Table::new(); - let report = get_report(report_name, filter)?; let working_set = WorkingSet::new(replica)?; + // Get the report from settings + let mut report = Report::from_config(settings.get(&format!("reports.{}", report_name))?) + .map_err(|e| format_err!("report.{}{}", report_name, e))?; + + // include any user-supplied filter conditions + report.filter = report.filter.intersect(filter); + // Get the tasks from the filter let mut tasks: Vec<_> = filtered_tasks(replica, &report.filter)?.collect(); diff --git a/cli/src/report.rs b/cli/src/report.rs index 370e48706..12ba0af99 100644 --- a/cli/src/report.rs +++ b/cli/src/report.rs @@ -1,6 +1,7 @@ //! This module contains the data structures used to define reports. -use crate::argparse::Filter; +use crate::argparse::{Condition, Filter}; +use failure::{bail, format_err, Fallible}; /// A report specifies a filter as well as a sort order and information about which /// task attributes to display @@ -68,3 +69,510 @@ pub(crate) enum SortBy { /// The task's description Description, } + +// Conversions from config::Value. Note that these cannot ergonomically use TryFrom/TryInto; see +// https://github.com/mehcode/config-rs/issues/162 + +impl Report { + /// Create a Report from a config value. This should be the `report.` value. + /// The error message begins with any additional path information, e.g., `.sort[1].sort_by: + /// ..`. + pub(crate) fn from_config(cfg: config::Value) -> Fallible { + let mut map = cfg.into_table().map_err(|e| format_err!(": {}", e))?; + let sort = if let Some(sort_array) = map.remove("sort") { + sort_array + .into_array() + .map_err(|e| format_err!(".sort: {}", e))? + .drain(..) + .enumerate() + .map(|(i, v)| Sort::from_config(v).map_err(|e| format_err!(".sort[{}]{}", i, e))) + .collect::>>()? + } else { + vec![] + }; + + let columns = map + .remove("columns") + .ok_or_else(|| format_err!(": 'columns' property is required"))? + .into_array() + .map_err(|e| format_err!(".columns: {}", e))? + .drain(..) + .enumerate() + .map(|(i, v)| Column::from_config(v).map_err(|e| format_err!(".columns[{}]{}", i, e))) + .collect::>>()?; + + let conditions = if let Some(conditions) = map.remove("filter") { + conditions + .into_array() + .map_err(|e| format_err!(".filter: {}", e))? + .drain(..) + .enumerate() + .map(|(i, v)| { + v.into_str() + .map_err(|e| e.into()) + .and_then(|s| Condition::parse_str(&s)) + .map_err(|e| format_err!(".filter[{}]: {}", i, e)) + }) + .collect::>>()? + } else { + vec![] + }; + + let filter = Filter { conditions }; + + if !map.is_empty() { + bail!(": unknown properties"); + } + + Ok(Report { + columns, + sort, + filter, + }) + } +} + +impl Column { + pub(crate) fn from_config(cfg: config::Value) -> Fallible { + let mut map = cfg.into_table().map_err(|e| format_err!(": {}", e))?; + let label = map + .remove("label") + .ok_or_else(|| format_err!(": 'label' property is required"))? + .into_str() + .map_err(|e| format_err!(".label: {}", e))?; + let property: config::Value = map + .remove("property") + .ok_or_else(|| format_err!(": 'property' property is required"))?; + let property = + Property::from_config(property).map_err(|e| format_err!(".property{}", e))?; + + if !map.is_empty() { + bail!(": unknown properties"); + } + + Ok(Column { label, property }) + } +} + +impl Property { + pub(crate) fn from_config(cfg: config::Value) -> Fallible { + let s = cfg.into_str().map_err(|e| format_err!(": {}", e))?; + Ok(match s.as_ref() { + "id" => Property::Id, + "uuid" => Property::Uuid, + "active" => Property::Active, + "description" => Property::Description, + "tags" => Property::Tags, + _ => bail!(": unknown property {}", s), + }) + } +} + +impl Sort { + pub(crate) fn from_config(cfg: config::Value) -> Fallible { + let mut map = cfg.into_table().map_err(|e| format_err!(": {}", e))?; + let ascending = match map.remove("ascending") { + Some(v) => v + .into_bool() + .map_err(|e| format_err!(".ascending: {}", e))?, + None => true, // default + }; + let sort_by: config::Value = map + .remove("sort_by") + .ok_or_else(|| format_err!(": 'sort_by' property is required"))?; + let sort_by = SortBy::from_config(sort_by).map_err(|e| format_err!(".sort_by{}", e))?; + + if !map.is_empty() { + bail!(": unknown properties"); + } + + Ok(Sort { ascending, sort_by }) + } +} + +impl SortBy { + pub(crate) fn from_config(cfg: config::Value) -> Fallible { + let s = cfg.into_str().map_err(|e| format_err!(": {}", e))?; + Ok(match s.as_ref() { + "id" => SortBy::Id, + "uuid" => SortBy::Uuid, + "description" => SortBy::Description, + _ => bail!(": unknown sort_by {}", s), + }) + } +} + +#[cfg(test)] +mod test { + use super::*; + use config::{Config, File, FileFormat, FileSourceString}; + use taskchampion::Status; + use textwrap::{dedent, indent}; + + fn config_from(cfg: &str) -> config::Value { + // wrap this in a "table" so that we can get any type of value at the top level. + let yaml = format!("val:\n{}", indent(&dedent(&cfg), " ")); + let mut settings = Config::new(); + let cfg_file: File = File::from_str(&yaml, FileFormat::Yaml); + settings.merge(cfg_file).unwrap(); + settings.cache.into_table().unwrap().remove("val").unwrap() + } + + #[test] + fn test_report_ok() { + let val = config_from( + " + filter: [] + sort: [] + columns: [] + filter: + - status:pending", + ); + let report = Report::from_config(val).unwrap(); + assert_eq!( + report.filter, + Filter { + conditions: vec![Condition::Status(Status::Pending),], + } + ); + assert_eq!(report.columns, vec![]); + assert_eq!(report.sort, vec![]); + } + + #[test] + fn test_report_no_sort() { + let val = config_from( + " + filter: [] + columns: []", + ); + let report = Report::from_config(val).unwrap(); + assert_eq!(report.sort, vec![]); + } + + #[test] + fn test_report_sort_not_array() { + let val = config_from( + " + filter: [] + sort: true + columns: []", + ); + assert_eq!( + &Report::from_config(val).unwrap_err().to_string(), + ".sort: invalid type: boolean `true`, expected an array" + ); + } + + #[test] + fn test_report_sort_error() { + let val = config_from( + " + filter: [] + sort: + - sort_by: id + - true + columns: []", + ); + assert!(&Report::from_config(val) + .unwrap_err() + .to_string() + .starts_with(".sort[1]")); + } + + #[test] + fn test_report_unknown_prop() { + let val = config_from( + " + columns: [] + filter: [] + sort: [] + nosuch: true + ", + ); + assert_eq!( + &Report::from_config(val).unwrap_err().to_string(), + ": unknown properties" + ); + } + + #[test] + fn test_report_no_columns() { + let val = config_from( + " + filter: [] + sort: []", + ); + assert_eq!( + &Report::from_config(val).unwrap_err().to_string(), + ": \'columns\' property is required" + ); + } + + #[test] + fn test_report_columns_not_array() { + let val = config_from( + " + filter: [] + sort: [] + columns: true", + ); + assert_eq!( + &Report::from_config(val).unwrap_err().to_string(), + ".columns: invalid type: boolean `true`, expected an array" + ); + } + + #[test] + fn test_report_column_error() { + let val = config_from( + " + filter: [] + sort: [] + columns: + - label: ID + property: id + - true", + ); + assert!(&Report::from_config(val) + .unwrap_err() + .to_string() + .starts_with(".columns[1]:")); + } + + #[test] + fn test_report_filter_not_array() { + let val = config_from( + " + filter: [] + sort: [] + columns: [] + filter: true", + ); + assert_eq!( + &Report::from_config(val).unwrap_err().to_string(), + ".filter: invalid type: boolean `true`, expected an array" + ); + } + + #[test] + fn test_report_filter_error() { + let val = config_from( + " + filter: [] + sort: [] + columns: [] + filter: + - nosuchfilter", + ); + assert!(&Report::from_config(val) + .unwrap_err() + .to_string() + .starts_with(".filter[0]: invalid filter condition:")); + } + + #[test] + fn test_column() { + let val = config_from( + " + label: ID + property: id", + ); + let column = Column::from_config(val).unwrap(); + assert_eq!( + column, + Column { + label: "ID".to_owned(), + property: Property::Id, + } + ); + } + + #[test] + fn test_column_unknown_prop() { + let val = config_from( + " + label: ID + property: id + nosuch: foo", + ); + assert_eq!( + &Column::from_config(val).unwrap_err().to_string(), + ": unknown properties" + ); + } + + #[test] + fn test_column_no_label() { + let val = config_from( + " + property: id", + ); + assert_eq!( + &Column::from_config(val).unwrap_err().to_string(), + ": 'label' property is required" + ); + } + + #[test] + fn test_column_invalid_label() { + let val = config_from( + " + label: [] + property: id", + ); + assert_eq!( + &Column::from_config(val).unwrap_err().to_string(), + ".label: invalid type: sequence, expected a string" + ); + } + + #[test] + fn test_column_no_property() { + let val = config_from( + " + label: ID", + ); + assert_eq!( + &Column::from_config(val).unwrap_err().to_string(), + ": 'property' property is required" + ); + } + + #[test] + fn test_column_invalid_property() { + let val = config_from( + " + label: ID + property: []", + ); + assert_eq!( + &Column::from_config(val).unwrap_err().to_string(), + ".property: invalid type: sequence, expected a string" + ); + } + + #[test] + fn test_property() { + let val = config_from("uuid"); + let prop = Property::from_config(val).unwrap(); + assert_eq!(prop, Property::Uuid); + } + + #[test] + fn test_property_invalid_type() { + let val = config_from("{}"); + assert_eq!( + &Property::from_config(val).unwrap_err().to_string(), + ": invalid type: map, expected a string" + ); + } + + #[test] + fn test_sort() { + let val = config_from( + " + ascending: false + sort_by: id", + ); + let sort = Sort::from_config(val).unwrap(); + assert_eq!( + sort, + Sort { + ascending: false, + sort_by: SortBy::Id, + } + ); + } + + #[test] + fn test_sort_no_ascending() { + let val = config_from( + " + sort_by: id", + ); + let sort = Sort::from_config(val).unwrap(); + assert_eq!( + sort, + Sort { + ascending: true, + sort_by: SortBy::Id, + } + ); + } + + #[test] + fn test_sort_unknown_prop() { + let val = config_from( + " + sort_by: id + nosuch: foo", + ); + assert_eq!( + &Sort::from_config(val).unwrap_err().to_string(), + ": unknown properties" + ); + } + + #[test] + fn test_sort_no_sort_by() { + let val = config_from( + " + ascending: true", + ); + assert_eq!( + &Sort::from_config(val).unwrap_err().to_string(), + ": 'sort_by' property is required" + ); + } + + #[test] + fn test_sort_invalid_ascending() { + let val = config_from( + " + sort_by: id + ascending: {}", + ); + assert_eq!( + &Sort::from_config(val).unwrap_err().to_string(), + ".ascending: invalid type: map, expected a boolean" + ); + } + + #[test] + fn test_sort_invalid_sort_by() { + let val = config_from( + " + sort_by: {}", + ); + assert_eq!( + &Sort::from_config(val).unwrap_err().to_string(), + ".sort_by: invalid type: map, expected a string" + ); + } + + #[test] + fn test_sort_by() { + let val = config_from("uuid"); + let prop = SortBy::from_config(val).unwrap(); + assert_eq!(prop, SortBy::Uuid); + } + + #[test] + fn test_sort_by_unknown() { + let val = config_from("nosuch"); + assert_eq!( + &SortBy::from_config(val).unwrap_err().to_string(), + ": unknown sort_by nosuch" + ); + } + + #[test] + fn test_sort_by_invalid_type() { + let val = config_from("{}"); + assert_eq!( + &SortBy::from_config(val).unwrap_err().to_string(), + ": invalid type: map, expected a string" + ); + } +} diff --git a/cli/src/settings.rs b/cli/src/settings.rs index 4d59b00be..88d972dc4 100644 --- a/cli/src/settings.rs +++ b/cli/src/settings.rs @@ -1,9 +1,40 @@ -use config::{Config, Environment, File, FileSourceFile}; +use config::{Config, Environment, File, FileFormat, FileSourceFile, FileSourceString}; use failure::Fallible; use std::env; use std::path::PathBuf; -pub(crate) fn read_settings() -> Fallible { +const DEFAULTS: &str = r#" +reports: + list: + sort: + - sort_by: uuid + columns: + - label: Id + property: id + - label: Description + property: description + - label: Active + property: active + - label: Tags + property: tags + next: + filter: + - "status:pending" + sort: + - sort_by: uuid + columns: + - label: Id + property: id + - label: Description + property: description + - label: Active + property: active + - label: Tags + property: tags +"#; + +/// Get the default settings for this application +pub(crate) fn default_settings() -> Fallible { let mut settings = Config::default(); // set up defaults @@ -25,6 +56,15 @@ pub(crate) fn read_settings() -> Fallible { )?; } + let defaults: File = File::from_str(DEFAULTS, FileFormat::Yaml); + settings.merge(defaults)?; + + Ok(settings) +} + +pub(crate) fn read_settings() -> Fallible { + let mut settings = default_settings()?; + // load either from the path in TASKCHAMPION_CONFIG, or from CONFIG_DIR/taskchampion if let Some(config_file) = env::var_os("TASKCHAMPION_CONFIG") { log::debug!("Loading configuration from {:?}", config_file);