From debeadbf3fc430480fab5e08ad8437b3e2a11adf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Sat, 6 Feb 2021 20:05:47 -0500 Subject: [PATCH] Soft rest arguments column path cohersions. (#3016) --- crates/nu-command/src/commands.rs | 5 +- crates/nu-command/src/commands/get.rs | 28 ++--- crates/nu-command/src/commands/select.rs | 14 ++- crates/nu-command/src/utils/arguments.rs | 93 ++++++++++++-- crates/nu-command/tests/commands/open.rs | 2 +- crates/nu-command/tests/commands/select.rs | 4 +- crates/nu-value-ext/src/lib.rs | 10 -- crates/nu-value-ext/src/tests.rs | 139 +++++++++++++++++++++ 8 files changed, 246 insertions(+), 49 deletions(-) create mode 100644 crates/nu-value-ext/src/tests.rs diff --git a/crates/nu-command/src/commands.rs b/crates/nu-command/src/commands.rs index a5ac12a949..05985b0f73 100644 --- a/crates/nu-command/src/commands.rs +++ b/crates/nu-command/src/commands.rs @@ -197,7 +197,7 @@ pub(crate) use from_xlsx::FromXLSX; pub(crate) use from_xml::FromXML; pub(crate) use from_yaml::FromYAML; pub(crate) use from_yaml::FromYML; -pub(crate) use get::Get; +pub(crate) use get::Command as Get; pub(crate) use group_by::Command as GroupBy; pub(crate) use group_by_date::GroupByDate; pub(crate) use hash_::{Hash, HashBase64}; @@ -299,7 +299,8 @@ mod tests { whole_stream_command(Move), whole_stream_command(Update), whole_stream_command(Empty), - //whole_stream_command(Select), + // whole_stream_command(Select), + // whole_stream_command(Get), // Str Command Suite whole_stream_command(Str), whole_stream_command(StrToDecimal), diff --git a/crates/nu-command/src/commands/get.rs b/crates/nu-command/src/commands/get.rs index 7e08dd655a..7d8bc5a43a 100644 --- a/crates/nu-command/src/commands/get.rs +++ b/crates/nu-command/src/commands/get.rs @@ -1,4 +1,5 @@ use crate::prelude::*; +use crate::utils::arguments::arguments; use indexmap::set::IndexSet; use log::trace; use nu_engine::WholeStreamCommand; @@ -10,22 +11,22 @@ use nu_protocol::{ use nu_source::HasFallibleSpan; use nu_value_ext::get_data_by_column_path; -pub struct Get; +pub struct Command; #[derive(Deserialize)] -pub struct GetArgs { - rest: Vec, +pub struct Arguments { + rest: Vec, } #[async_trait] -impl WholeStreamCommand for Get { +impl WholeStreamCommand for Command { fn name(&self) -> &str { "get" } fn signature(&self) -> Signature { Signature::build("get").rest( - SyntaxShape::ColumnPath, + SyntaxShape::Any, "optionally return additional data by path", ) } @@ -55,7 +56,9 @@ impl WholeStreamCommand for Get { } pub async fn get(args: CommandArgs) -> Result { - let (GetArgs { rest: column_paths }, mut input) = args.process().await?; + let (Arguments { mut rest }, mut input) = args.process().await?; + let (column_paths, _) = arguments(&mut rest)?; + if column_paths.is_empty() { let vec = input.drain_vec().await; @@ -255,16 +258,3 @@ pub fn get_column_from_row_error( )), } } - -#[cfg(test)] -mod tests { - use super::Get; - use super::ShellError; - - #[test] - fn examples_work_as_expected() -> Result<(), ShellError> { - use crate::examples::test as test_examples; - - Ok(test_examples(Get {})?) - } -} diff --git a/crates/nu-command/src/commands/select.rs b/crates/nu-command/src/commands/select.rs index b461f3007f..f8ddb49796 100644 --- a/crates/nu-command/src/commands/select.rs +++ b/crates/nu-command/src/commands/select.rs @@ -3,10 +3,11 @@ use crate::utils::arguments::arguments; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; use nu_protocol::{ - hir::CapturedBlock, ColumnPath, PathMember, Primitive, ReturnSuccess, Signature, SyntaxShape, - TaggedDictBuilder, UnspannedPathMember, UntaggedValue, Value, + PathMember, Primitive, ReturnSuccess, Signature, SyntaxShape, TaggedDictBuilder, + UnspannedPathMember, UntaggedValue, Value, }; use nu_value_ext::{as_string, get_data_by_column_path}; + #[derive(Deserialize)] struct Arguments { rest: Vec, @@ -51,7 +52,7 @@ impl WholeStreamCommand for Command { async fn select(args: CommandArgs) -> Result { let name = args.call_info.name_tag.clone(); let (Arguments { mut rest }, mut input) = args.process().await?; - let (columns, _): (Vec, Option>) = arguments(&mut rest)?; + let (columns, _) = arguments(&mut rest)?; if columns.is_empty() { return Err(ShellError::labeled_error( @@ -140,15 +141,16 @@ async fn select(args: CommandArgs) -> Result { let mut out = TaggedDictBuilder::new(name.clone()); for k in &keys { + let new_key = k.replace(".", "_"); let nothing = UntaggedValue::Primitive(Primitive::Nothing).into_untagged_value(); let subsets = bring_back.get(k); match subsets { Some(set) => match set.get(current) { - Some(row) => out.insert_untagged(k, row.get_data(k).borrow().clone()), - None => out.insert_untagged(k, nothing.clone()), + Some(row) => out.insert_untagged(new_key, row.get_data(k).borrow().clone()), + None => out.insert_untagged(new_key, nothing.clone()), }, - None => out.insert_untagged(k, nothing.clone()), + None => out.insert_untagged(new_key, nothing.clone()), } } diff --git a/crates/nu-command/src/utils/arguments.rs b/crates/nu-command/src/utils/arguments.rs index 498c65e94e..06998207c3 100644 --- a/crates/nu-command/src/utils/arguments.rs +++ b/crates/nu-command/src/utils/arguments.rs @@ -1,9 +1,15 @@ use indexmap::IndexSet; use nu_errors::ShellError; use nu_protocol::{hir::CapturedBlock, ColumnPath, UntaggedValue, Value}; -use nu_source::Tagged; use nu_value_ext::ValueExt; +/// Commands can be used in block form (passing a block) and +/// in the majority of cases we are also interested in accepting +/// column names along with it. +/// +/// This aids with commands that take rest arguments +/// that need to be column names and an optional block as last +/// argument. pub fn arguments( rest: &mut Vec, ) -> Result<(Vec, Option>), ShellError> { @@ -15,9 +21,14 @@ pub fn arguments( let mut default = None; for argument in columns.drain(..) { - let Tagged { item: path, .. } = argument.as_column_path()?; - - column_paths.push(path); + match &argument.value { + UntaggedValue::Table(values) => { + column_paths.extend(collect_as_column_paths(&values)?); + } + _ => { + column_paths.push(argument.as_column_path()?.item); + } + } } match last_argument { @@ -25,13 +36,77 @@ pub fn arguments( value: UntaggedValue::Block(call), .. }) => default = Some(call), - Some(other) => { - let Tagged { item: path, .. } = other.as_column_path()?; - - column_paths.push(path); - } + Some(other) => match &other.value { + UntaggedValue::Table(values) => { + column_paths.extend(collect_as_column_paths(&values)?); + } + _ => { + column_paths.push(other.as_column_path()?.item); + } + }, None => {} }; Ok((column_paths, default)) } + +fn collect_as_column_paths(values: &[Value]) -> Result, ShellError> { + let mut out = vec![]; + + for name in values { + out.push(name.as_column_path()?.item); + } + + Ok(out) +} + +#[cfg(test)] +mod tests { + use super::arguments; + use nu_test_support::value::*; + use nu_value_ext::ValueExt; + + #[test] + fn arguments_test() -> Result<(), Box> { + // cmd name + let arg1 = string("name"); + let expected = string("name").as_column_path()?.item; + + let (args, _) = arguments(&mut vec![arg1])?; + + assert_eq!(args[0], expected); + + Ok(()) + } + + #[test] + fn arguments_test_2() -> Result<(), Box> { + // cmd name [type] + let arg1 = string("name"); + let arg2 = table(&vec![string("type")]); + + let expected = vec![ + string("name").as_column_path()?.item, + string("type").as_column_path()?.item, + ]; + + assert_eq!(arguments(&mut vec![arg1, arg2])?.0, expected); + + Ok(()) + } + + #[test] + fn arguments_test_3() -> Result<(), Box> { + // cmd [name type] + let arg1 = table(&vec![string("name"), string("type")]); + + let expected = vec![ + string("name").as_column_path()?.item, + string("type").as_column_path()?.item, + ]; + + assert_eq!(arguments(&mut vec![arg1])?.0, expected); + + Ok(()) + } +} diff --git a/crates/nu-command/tests/commands/open.rs b/crates/nu-command/tests/commands/open.rs index 4aec9f2a25..6f39b0772f 100644 --- a/crates/nu-command/tests/commands/open.rs +++ b/crates/nu-command/tests/commands/open.rs @@ -210,7 +210,7 @@ fn parses_ini() { fn parses_utf16_ini() { let actual = nu!( cwd: "tests/fixtures/formats", - "open utf16.ini | get '.ShellClassInfo' | get IconIndex" + "open utf16.ini | rename info | get info | get IconIndex" ); assert_eq!(actual.out, "-236") diff --git a/crates/nu-command/tests/commands/select.rs b/crates/nu-command/tests/commands/select.rs index 87b1708be9..0af280721c 100644 --- a/crates/nu-command/tests/commands/select.rs +++ b/crates/nu-command/tests/commands/select.rs @@ -55,8 +55,8 @@ fn complex_nested_columns() { r#" open los_tres_caballeros.json | select nu."0xATYKARNU" nu.committers.name nu.releases.version - | where "nu.releases.version" > "0.8" - | get "nu.releases.version" + | where nu_releases_version > "0.8" + | get nu_releases_version "# )); diff --git a/crates/nu-value-ext/src/lib.rs b/crates/nu-value-ext/src/lib.rs index 1384bf88b9..caff9641ee 100644 --- a/crates/nu-value-ext/src/lib.rs +++ b/crates/nu-value-ext/src/lib.rs @@ -628,16 +628,6 @@ pub fn replace_data_at_column_path( pub fn as_column_path(value: &Value) -> Result, ShellError> { match &value.value { - UntaggedValue::Table(table) => { - let mut out: Vec = vec![]; - - for item in table { - out.push(as_path_member(item)?); - } - - Ok(ColumnPath::new(out).tagged(&value.tag)) - } - UntaggedValue::Primitive(Primitive::String(s)) => { let s = s.to_string().spanned(value.tag.span); diff --git a/crates/nu-value-ext/src/tests.rs b/crates/nu-value-ext/src/tests.rs new file mode 100644 index 0000000000..7dd85ee0ef --- /dev/null +++ b/crates/nu-value-ext/src/tests.rs @@ -0,0 +1,139 @@ +use super::*; +use nu_test_support::value::*; + +use indexmap::indexmap; + +#[test] +fn forgiving_insertion_test_1() { + let field_path = column_path("crate.version").as_column_path().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.item, 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("things.0").as_column_path().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.item, 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("color_config.arepa_color") + .as_column_path() + .unwrap(); + let pizza_path = column_path("things.0").as_column_path().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.item, + 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.item, entry) + .unwrap() + .get_data_by_column_path(&pizza_path, Box::new(error_callback("things.0"))) + .unwrap(), + *string("Arepas de Yehuda") + ); +} + +#[test] +fn get_row_data_by_key() { + let row = row(indexmap! { + "lines".to_string() => int(0), + "words".to_string() => int(7), + }); + assert_eq!( + row.get_data_by_key("lines".spanned_unknown()).unwrap(), + int(0) + ); + assert!(row.get_data_by_key("chars".spanned_unknown()).is_none()); +} + +#[test] +fn get_table_data_by_key() { + let row1 = row(indexmap! { + "lines".to_string() => int(0), + "files".to_string() => int(10), + }); + + let row2 = row(indexmap! { + "files".to_string() => int(1) + }); + + let table_value = table(&[row1, row2]); + assert_eq!( + table_value + .get_data_by_key("files".spanned_unknown()) + .unwrap(), + table(&[int(10), int(1)]) + ); + assert_eq!( + table_value + .get_data_by_key("chars".spanned_unknown()) + .unwrap(), + table(&[nothing(), nothing()]) + ); +} \ No newline at end of file