From 973a8ee8f3e3de39da9e7917139a184aac5cc627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Thu, 8 Oct 2020 20:04:19 -0500 Subject: [PATCH] Allow config to work with column paths. (#2653) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nu has many commands that allow the nuño to customize behavior such as UI and behavior. Today, coloring can be customized, the line editor, and other things. The more options there are, the higher the complexity in managing them. To mitigate this Nu can store configuration options as nested properties. But to add and edit them can be taxing. With column path support we can work with them easier. --- Cargo.lock | 6 + crates/nu-cli/src/commands/config/get.rs | 23 +-- crates/nu-cli/src/commands/config/set.rs | 64 ++++-- crates/nu-data/Cargo.toml | 1 + crates/nu-data/src/base.rs | 35 +--- crates/nu-protocol/src/value.rs | 5 + crates/nu-test-support/Cargo.toml | 4 + crates/nu-test-support/src/lib.rs | 1 + crates/nu-test-support/src/value.rs | 50 +++++ crates/nu-value-ext/Cargo.toml | 3 +- crates/nu-value-ext/src/lib.rs | 241 ++++++++++++++++++++++- 11 files changed, 361 insertions(+), 72 deletions(-) create mode 100644 crates/nu-test-support/src/value.rs diff --git a/Cargo.lock b/Cargo.lock index a32ab7bc23..8530a5d156 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3008,6 +3008,7 @@ dependencies = [ "nu-protocol", "nu-source", "nu-table", + "nu-test-support", "nu-value-ext", "num-bigint 0.3.0", "num-traits 0.2.12", @@ -3116,12 +3117,16 @@ dependencies = [ name = "nu-test-support" version = "0.20.0" dependencies = [ + "chrono", "dunce", "getset", "glob", "indexmap", + "nu-errors", "nu-protocol", "nu-source", + "nu-value-ext", + "num-bigint 0.3.0", "tempfile", ] @@ -3134,6 +3139,7 @@ dependencies = [ "nu-errors", "nu-protocol", "nu-source", + "nu-test-support", "num-traits 0.2.12", ] diff --git a/crates/nu-cli/src/commands/config/get.rs b/crates/nu-cli/src/commands/config/get.rs index c2f19b26c2..e70bf90faa 100644 --- a/crates/nu-cli/src/commands/config/get.rs +++ b/crates/nu-cli/src/commands/config/get.rs @@ -2,14 +2,13 @@ use crate::command_registry::CommandRegistry; use crate::commands::WholeStreamCommand; use crate::prelude::*; use nu_errors::ShellError; -use nu_protocol::{ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value}; -use nu_source::Tagged; +use nu_protocol::{ColumnPath, ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value}; pub struct SubCommand; #[derive(Deserialize)] pub struct GetArgs { - get: Tagged, + path: ColumnPath, } #[async_trait] @@ -21,7 +20,7 @@ impl WholeStreamCommand for SubCommand { fn signature(&self) -> Signature { Signature::build("config get").required( "get", - SyntaxShape::Any, + SyntaxShape::ColumnPath, "value to get from the config", ) } @@ -51,17 +50,14 @@ pub async fn get( args: CommandArgs, registry: &CommandRegistry, ) -> Result { - let name_span = args.call_info.name_tag.clone(); - let (GetArgs { get }, _) = args.process(®istry).await?; + let name_tag = args.call_info.name_tag.clone(); + let (GetArgs { path }, _) = args.process(®istry).await?; // NOTE: None because we are not loading a new config file, we just want to read from the // existing config - let result = nu_data::config::read(name_span, &None)?; + let result = UntaggedValue::row(nu_data::config::read(&name_tag, &None)?).into_value(&name_tag); - let key = get.to_string(); - let value = result - .get(&key) - .ok_or_else(|| ShellError::labeled_error("Missing key in config", "key", get.tag()))?; + let value = crate::commands::get::get_column_path(&path, &result)?; Ok(match value { Value { @@ -75,9 +71,6 @@ pub async fn get( futures::stream::iter(list).to_output_stream() } - x => { - let x = x.clone(); - OutputStream::one(ReturnSuccess::value(x)) - } + x => OutputStream::one(ReturnSuccess::value(x)), }) } diff --git a/crates/nu-cli/src/commands/config/set.rs b/crates/nu-cli/src/commands/config/set.rs index 7d534c1f5a..52743fa456 100644 --- a/crates/nu-cli/src/commands/config/set.rs +++ b/crates/nu-cli/src/commands/config/set.rs @@ -2,14 +2,13 @@ use crate::command_registry::CommandRegistry; use crate::commands::WholeStreamCommand; use crate::prelude::*; use nu_errors::ShellError; -use nu_protocol::{ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value}; -use nu_source::Tagged; +use nu_protocol::{ColumnPath, ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value}; pub struct SubCommand; #[derive(Deserialize)] pub struct SetArgs { - key: Tagged, + path: ColumnPath, value: Value, } @@ -21,7 +20,7 @@ impl WholeStreamCommand for SubCommand { fn signature(&self) -> Signature { Signature::build("config set") - .required("key", SyntaxShape::String, "variable name to set") + .required("key", SyntaxShape::ColumnPath, "variable name to set") .required("value", SyntaxShape::Any, "value to use") } @@ -38,11 +37,28 @@ impl WholeStreamCommand for SubCommand { } fn examples(&self) -> Vec { - vec![Example { - description: "Set nonzero_exit_errors to true", - example: "config set nonzero_exit_errors $true", - result: None, - }] + vec![ + Example { + description: "Set auto pivoting", + example: "config set pivot_mode always", + result: None, + }, + Example { + description: "Set line editor options", + example: "config set line_editor [[edit_mode, completion_type]; [emacs circular]]", + result: None, + }, + Example { + description: "Set coloring options", + example: "config set color_config [[header_align header_bold]; [left $true]]", + result: None, + }, + Example { + description: "Set nested options", + example: "config set color_config.header_color white", + result: None, + }, + ] } } @@ -50,18 +66,32 @@ pub async fn set( args: CommandArgs, registry: &CommandRegistry, ) -> Result { - let name_span = args.call_info.name_tag.clone(); - let (SetArgs { key, value }, _) = args.process(®istry).await?; + let name_tag = args.call_info.name_tag.clone(); + let (SetArgs { path, mut value }, _) = args.process(®istry).await?; // NOTE: None because we are not loading a new config file, we just want to read from the // existing config - let mut result = nu_data::config::read(name_span, &None)?; + let raw_entries = nu_data::config::read(&name_tag, &None)?; + let configuration = UntaggedValue::row(raw_entries).into_value(&name_tag); - result.insert(key.to_string(), value.clone()); + if let UntaggedValue::Table(rows) = &value.value { + if rows.len() == 1 && rows[0].is_row() { + value = rows[0].clone(); + } + } - config::write(&result, &None)?; + match configuration.forgiving_insert_data_at_column_path(&path, value) { + Ok(Value { + value: UntaggedValue::Row(changes), + .. + }) => { + config::write(&changes.entries, &None)?; - Ok(OutputStream::one(ReturnSuccess::value( - UntaggedValue::Row(result.into()).into_value(&value.tag), - ))) + Ok(OutputStream::one(ReturnSuccess::value( + UntaggedValue::Row(changes).into_value(name_tag), + ))) + } + Ok(_) => Ok(OutputStream::empty()), + Err(reason) => Err(reason), + } } diff --git a/crates/nu-data/Cargo.toml b/crates/nu-data/Cargo.toml index 1304cd2468..97bd2d5196 100644 --- a/crates/nu-data/Cargo.toml +++ b/crates/nu-data/Cargo.toml @@ -33,6 +33,7 @@ nu-protocol = {version = "0.20.0", path = "../nu-protocol"} nu-source = {version = "0.20.0", path = "../nu-source"} nu-table = {version = "0.20.0", path = "../nu-table"} nu-value-ext = {version = "0.20.0", path = "../nu-value-ext"} +nu-test-support = {version = "0.20.0", path = "../nu-test-support"} [target.'cfg(unix)'.dependencies] users = "0.10.0" diff --git a/crates/nu-data/src/base.rs b/crates/nu-data/src/base.rs index 96b11138ec..ff5052760c 100644 --- a/crates/nu-data/src/base.rs +++ b/crates/nu-data/src/base.rs @@ -165,38 +165,13 @@ pub fn coerce_compare_primitive( } #[cfg(test)] mod tests { - use indexmap::{indexmap, IndexMap}; use nu_errors::ShellError; - use nu_protocol::{ColumnPath as ColumnPathValue, PathMember, UntaggedValue, Value}; - use nu_source::*; - use nu_value_ext::{as_column_path, ValueExt}; - use num_bigint::BigInt; + use nu_protocol::UntaggedValue; + use nu_source::SpannedItem; + use nu_test_support::value::*; + use nu_value_ext::ValueExt; - fn string(input: impl Into) -> Value { - crate::utils::helpers::string(input) - } - - fn int(input: impl Into) -> Value { - crate::utils::helpers::int(input) - } - - fn row(entries: IndexMap) -> Value { - crate::utils::helpers::row(entries) - } - - fn table(list: &[Value]) -> Value { - crate::utils::helpers::table(list) - } - - fn error_callback( - reason: &'static str, - ) -> impl FnOnce(&Value, &PathMember, ShellError) -> ShellError { - move |_obj_source, _column_path_tried, _err| ShellError::unimplemented(reason) - } - - fn column_path(paths: &[Value]) -> Result, ShellError> { - as_column_path(&table(paths)) - } + use indexmap::indexmap; #[test] fn gets_matching_field_from_a_row() -> Result<(), ShellError> { diff --git a/crates/nu-protocol/src/value.rs b/crates/nu-protocol/src/value.rs index 90c55c98a0..28fcdf1b0a 100644 --- a/crates/nu-protocol/src/value.rs +++ b/crates/nu-protocol/src/value.rs @@ -92,6 +92,11 @@ impl UntaggedValue { matches!(self, UntaggedValue::Table(_)) } + /// Returns true if this value represents a row + pub fn is_row(&self) -> bool { + matches!(self, UntaggedValue::Row(_)) + } + /// Returns true if this value represents a string pub fn is_string(&self) -> bool { matches!(self, UntaggedValue::Primitive(Primitive::String(_))) diff --git a/crates/nu-test-support/Cargo.toml b/crates/nu-test-support/Cargo.toml index 2877449c72..820a4f42c8 100644 --- a/crates/nu-test-support/Cargo.toml +++ b/crates/nu-test-support/Cargo.toml @@ -12,11 +12,15 @@ doctest = false [dependencies] nu-protocol = {path = "../nu-protocol", version = "0.20.0"} nu-source = {path = "../nu-source", version = "0.20.0"} +nu-errors = {version = "0.20.0", path = "../nu-errors"} +nu-value-ext = {version = "0.20.0", path = "../nu-value-ext"} +chrono = "0.4.15" dunce = "1.0.1" getset = "0.1.1" glob = "0.3.0" indexmap = {version = "1.6.0", features = ["serde-1"]} tempfile = "3.1.0" +num-bigint = {version = "0.3.0", features = ["serde"]} [build-dependencies] diff --git a/crates/nu-test-support/src/lib.rs b/crates/nu-test-support/src/lib.rs index 86ecd29f10..c022af692d 100644 --- a/crates/nu-test-support/src/lib.rs +++ b/crates/nu-test-support/src/lib.rs @@ -2,6 +2,7 @@ pub mod commands; pub mod fs; pub mod macros; pub mod playground; +pub mod value; pub fn pipeline(commands: &str) -> String { commands diff --git a/crates/nu-test-support/src/value.rs b/crates/nu-test-support/src/value.rs new file mode 100644 index 0000000000..3f3cc0cbbe --- /dev/null +++ b/crates/nu-test-support/src/value.rs @@ -0,0 +1,50 @@ +use chrono::{DateTime, NaiveDate, Utc}; +use indexmap::IndexMap; +use nu_errors::ShellError; +use nu_protocol::{ColumnPath, PathMember, Primitive, UntaggedValue, Value}; +use nu_source::{Span, Tagged, TaggedItem}; +use nu_value_ext::as_column_path; +use num_bigint::BigInt; + +pub fn int(s: impl Into) -> Value { + UntaggedValue::int(s).into_untagged_value() +} + +pub fn decimal_from_float(f: f64, span: Span) -> Value { + UntaggedValue::decimal_from_float(f, span).into_untagged_value() +} + +pub fn string(input: impl Into) -> Value { + UntaggedValue::string(input.into()).into_untagged_value() +} + +pub fn row(entries: IndexMap) -> Value { + UntaggedValue::row(entries).into_untagged_value() +} + +pub fn table(list: &[Value]) -> Value { + UntaggedValue::table(list).into_untagged_value() +} + +pub fn date(input: impl Into) -> Value { + let key = input.into().tagged_unknown(); + + let date = NaiveDate::parse_from_str(key.borrow_tagged().item, "%Y-%m-%d") + .expect("date from string failed"); + + UntaggedValue::Primitive(Primitive::Date(DateTime::::from_utc( + date.and_hms(12, 34, 56), + Utc, + ))) + .into_untagged_value() +} + +pub fn column_path(paths: &[Value]) -> Result, ShellError> { + as_column_path(&table(paths)) +} + +pub fn error_callback( + reason: &'static str, +) -> impl FnOnce(&Value, &PathMember, ShellError) -> ShellError { + move |_obj_source, _column_path_tried, _err| ShellError::unimplemented(reason) +} diff --git a/crates/nu-value-ext/Cargo.toml b/crates/nu-value-ext/Cargo.toml index 0cc453108e..2c6fa925f4 100644 --- a/crates/nu-value-ext/Cargo.toml +++ b/crates/nu-value-ext/Cargo.toml @@ -18,4 +18,5 @@ indexmap = {version = "1.6.0", features = ["serde-1"]} itertools = "0.9.0" num-traits = "0.2.12" -[build-dependencies] +[dev-dependencies] +nu-test-support = {path = "../nu-test-support", version = "0.20.0"} \ No newline at end of file diff --git a/crates/nu-value-ext/src/lib.rs b/crates/nu-value-ext/src/lib.rs index 08f1f55cdb..6dbf625038 100644 --- a/crates/nu-value-ext/src/lib.rs +++ b/crates/nu-value-ext/src/lib.rs @@ -1,3 +1,4 @@ +use indexmap::indexmap; use indexmap::set::IndexSet; use itertools::Itertools; use nu_errors::{ExpectedRange, ShellError}; @@ -31,6 +32,11 @@ pub trait ValueExt { member: &PathMember, new_value: Value, ) -> Result<(), ShellError>; + fn forgiving_insert_data_at_column_path( + &self, + split_path: &ColumnPath, + new_value: Value, + ) -> Result; fn insert_data_at_column_path( &self, split_path: &ColumnPath, @@ -99,6 +105,14 @@ impl ValueExt for Value { insert_data_at_column_path(self, split_path, new_value) } + fn forgiving_insert_data_at_column_path( + &self, + split_path: &ColumnPath, + new_value: Value, + ) -> Result { + forgiving_insert_data_at_column_path(self, split_path, new_value) + } + fn replace_data_at_column_path( &self, split_path: &ColumnPath, @@ -217,7 +231,7 @@ where pub fn swap_data_by_column_path( value: &Value, path: &ColumnPath, - get_replacement: F, + callback: F, ) -> Result where F: FnOnce(&Value) -> Result, @@ -357,8 +371,8 @@ where error }); - let to_replace = to_replace?; - let replacement = get_replacement(&to_replace)?; + let old_value = to_replace?; + let replacement = callback(&old_value)?; value .replace_data_at_column_path(&path, replacement) @@ -458,6 +472,101 @@ pub fn insert_data_at_member( } } +pub fn missing_path_members_by_column_path(value: &Value, path: &ColumnPath) -> Option { + let mut current = value.clone(); + + for (idx, p) in path.iter().enumerate() { + if let Ok(value) = get_data_by_member(¤t, p) { + current = value; + } else { + return Some(idx); + } + } + + None +} + +pub fn forgiving_insert_data_at_column_path( + value: &Value, + split_path: &ColumnPath, + new_value: Value, +) -> Result { + let mut original = value.clone(); + + if let Some(missed_at) = missing_path_members_by_column_path(value, split_path) { + let mut paths = split_path.iter().skip(missed_at + 1).collect::>(); + paths.reverse(); + + let mut candidate = new_value; + + for member in paths.iter() { + match &member.unspanned { + UnspannedPathMember::String(column_name) => { + candidate = + UntaggedValue::row(indexmap! { column_name.into() => candidate.clone()}) + .into_value(&candidate.tag) + } + UnspannedPathMember::Int(int) => { + let mut rows = vec![]; + let size = int.to_usize().unwrap_or_else(|| 0); + + for _ in 0..=size { + rows.push( + UntaggedValue::Primitive(Primitive::Nothing).into_value(&candidate.tag), + ); + } + rows.push(candidate.clone()); + candidate = UntaggedValue::Table(rows).into_value(&candidate.tag); + } + } + } + + let cp = ColumnPath::new( + split_path + .iter() + .cloned() + .take(split_path.members().len() - missed_at + 1) + .collect::>(), + ); + + if missed_at == 0 { + let current: &mut Value = &mut original; + insert_data_at_member(current, &cp.members()[0], candidate)?; + return Ok(original); + } + + if value + .get_data_by_column_path(&cp, Box::new(move |_, _, err| err)) + .is_ok() + { + return insert_data_at_column_path(&value, &cp, candidate); + } else if let Some((last, front)) = cp.split_last() { + let mut current: &mut Value = &mut original; + + for member in front { + let type_name = current.spanned_type_name(); + + current = get_mut_data_by_member(current, &member).ok_or_else(|| { + ShellError::missing_property( + member.plain_string(std::usize::MAX).spanned(member.span), + type_name, + ) + })? + } + + insert_data_at_member(current, &last, candidate)?; + + return Ok(original); + } else { + return Err(ShellError::untagged_runtime_error( + "Internal error: could not split column path correctly", + )); + } + } + + insert_data_at_column_path(&value, split_path, new_value) +} + pub fn insert_data_at_column_path( value: &Value, split_path: &ColumnPath, @@ -592,12 +701,24 @@ fn insert_data_at_index( index: Tagged, new_value: Value, ) -> Result<(), ShellError> { - if list.len() >= index.item { - Err(ShellError::range_error( - 0..(list.len()), - &format_args!("{}", index.item).spanned(index.tag.span), - "insert at index", - )) + if index.item >= list.len() { + if index.item == list.len() { + list.push(new_value); + return Ok(()); + } + + let mut idx = list.len(); + + loop { + list.push(UntaggedValue::Primitive(Primitive::Nothing).into_value(&new_value.tag)); + + idx += 1; + + if idx == index.item { + list.push(new_value); + return Ok(()); + } + } } else { list[index.item] = new_value; Ok(()) @@ -689,3 +810,105 @@ pub(crate) fn get_mut_data_by_member<'value>( _ => None, } } + +#[cfg(test)] +mod tests { + use super::*; + use nu_test_support::value::*; + + use indexmap::indexmap; + + #[test] + fn forgiving_insertion_test_1() { + let field_path = column_path(&[string("crate"), string("version")]).unwrap(); + + let version = string("nuno"); + + let value = UntaggedValue::row(indexmap! { + "package".into() => + row(indexmap! { + "name".into() => string("nu"), + "version".into() => string("0.20.0") + }) + }); + + assert_eq!( + *value + .into_untagged_value() + .forgiving_insert_data_at_column_path(&field_path, version) + .unwrap() + .get_data_by_column_path(&field_path, Box::new(error_callback("crate.version"))) + .unwrap(), + *string("nuno") + ); + } + + #[test] + fn forgiving_insertion_test_2() { + let field_path = column_path(&[string("things"), int(0)]).unwrap(); + + let version = string("arepas"); + + let value = UntaggedValue::row(indexmap! { + "pivot_mode".into() => string("never"), + "things".into() => table(&[string("frijoles de Andrés"), int(1)]), + "color_config".into() => + row(indexmap! { + "header_align".into() => string("left"), + "index_color".into() => string("cyan_bold") + }) + }); + + assert_eq!( + *value + .into_untagged_value() + .forgiving_insert_data_at_column_path(&field_path, version) + .unwrap() + .get_data_by_column_path(&field_path, Box::new(error_callback("things.0"))) + .unwrap(), + *string("arepas") + ); + } + + #[test] + fn forgiving_insertion_test_3() { + let field_path = column_path(&[string("color_config"), string("arepa_color")]).unwrap(); + let pizza_path = column_path(&[string("things"), int(0)]).unwrap(); + + let entry = string("amarillo"); + + let value = UntaggedValue::row(indexmap! { + "pivot_mode".into() => string("never"), + "things".into() => table(&[string("Arepas de Yehuda"), int(1)]), + "color_config".into() => + row(indexmap! { + "header_align".into() => string("left"), + "index_color".into() => string("cyan_bold") + }) + }); + + assert_eq!( + *value + .clone() + .into_untagged_value() + .forgiving_insert_data_at_column_path(&field_path, entry.clone()) + .unwrap() + .get_data_by_column_path( + &field_path, + Box::new(error_callback("color_config.arepa_color")) + ) + .unwrap(), + *string("amarillo") + ); + + assert_eq!( + *value + .into_untagged_value() + .forgiving_insert_data_at_column_path(&field_path, entry) + .unwrap() + .get_data_by_column_path(&pizza_path, Box::new(error_callback("things.0"))) + .unwrap(), + *string("Arepas de Yehuda") + ); + } +}