From 440e12abc445b85ee7bb7c282645fca8e7301f13 Mon Sep 17 00:00:00 2001 From: JT <jonathandturner@users.noreply.github.com> Date: Wed, 9 Jun 2021 18:07:54 +1200 Subject: [PATCH] Fix #3582 (#3583) * Fix #3582 * Fix empty? * Fix parsing types --- crates/nu-command/src/commands/empty.rs | 33 +++--- crates/nu-command/src/commands/get.rs | 7 +- crates/nu-command/src/commands/select.rs | 13 +- crates/nu-command/src/commands/sort_by.rs | 2 +- .../src/commands/str_/to_datetime.rs | 19 ++- .../src/commands/str_/to_integer.rs | 17 ++- crates/nu-command/src/utils.rs | 1 - crates/nu-command/src/utils/arguments.rs | 112 ------------------ crates/nu-command/tests/commands/empty.rs | 8 +- 9 files changed, 47 insertions(+), 165 deletions(-) delete mode 100644 crates/nu-command/src/utils/arguments.rs diff --git a/crates/nu-command/src/commands/empty.rs b/crates/nu-command/src/commands/empty.rs index 4151357933..958d712727 100644 --- a/crates/nu-command/src/commands/empty.rs +++ b/crates/nu-command/src/commands/empty.rs @@ -7,7 +7,6 @@ use nu_protocol::{ SyntaxShape, UntaggedValue, Value, }; -use crate::utils::arguments::arguments; use nu_value_ext::{as_string, ValueExt}; pub struct Command; @@ -18,10 +17,17 @@ impl WholeStreamCommand for Command { } fn signature(&self) -> Signature { - Signature::build("empty?").rest( - SyntaxShape::Any, - "the names of the columns to check emptiness. Pass an optional block to replace if empty", - ) + Signature::build("empty?") + .rest( + SyntaxShape::ColumnPath, + "the names of the columns to check emptiness", + ) + .named( + "block", + SyntaxShape::Block, + "an optional block to replace if empty", + Some('b'), + ) } fn usage(&self) -> &str { @@ -58,7 +64,7 @@ impl WholeStreamCommand for Command { ), },Example { description: "use a block if setting the empty cell contents is wanted", - example: "echo [[2020/04/16 2020/07/10 2020/11/16]; ['' [27] [37]]] | empty? 2020/04/16 { [33 37] }", + example: "echo [[2020/04/16 2020/07/10 2020/11/16]; ['' [27] [37]]] | empty? 2020/04/16 -b { [33 37] }", result: Some( vec![ UntaggedValue::row(indexmap! { @@ -79,11 +85,10 @@ fn is_empty(args: CommandArgs) -> Result<ActionStream, ShellError> { let name_tag = Arc::new(args.call_info.name_tag.clone()); let context = Arc::new(EvaluationContext::from_args(&args)); let args = args.evaluate_once()?; - let mut rest = args.rest(0)?; - let (columns, default_block): (Vec<ColumnPath>, Option<Box<CapturedBlock>>) = - arguments(&mut rest)?; + let block: Option<CapturedBlock> = args.get_flag("block")?; + let columns: Vec<ColumnPath> = args.rest(0)?; + let input = args.input; - let default_block = Arc::new(default_block); if input.is_empty() { let stream = vec![UntaggedValue::Primitive(Primitive::Nothing).into_value(tag)].into_iter(); @@ -92,10 +97,9 @@ fn is_empty(args: CommandArgs) -> Result<ActionStream, ShellError> { .map(move |input| { let tag = name_tag.clone(); let context = context.clone(); - let block = default_block.clone(); let columns = vec![]; - match process_row(context, input, block, columns, tag) { + match process_row(context, input, &block, columns, tag) { Ok(s) => s, Err(e) => ActionStream::one(Err(e)), } @@ -108,10 +112,9 @@ fn is_empty(args: CommandArgs) -> Result<ActionStream, ShellError> { .map(move |input| { let tag = name_tag.clone(); let context = context.clone(); - let block = default_block.clone(); let columns = columns.clone(); - match process_row(context, input, block, columns, tag) { + match process_row(context, input, &block, columns, tag) { Ok(s) => s, Err(e) => ActionStream::one(Err(e)), } @@ -123,7 +126,7 @@ fn is_empty(args: CommandArgs) -> Result<ActionStream, ShellError> { fn process_row( context: Arc<EvaluationContext>, input: Value, - default_block: Arc<Option<Box<CapturedBlock>>>, + default_block: &Option<CapturedBlock>, column_paths: Vec<ColumnPath>, tag: Arc<Tag>, ) -> Result<ActionStream, ShellError> { diff --git a/crates/nu-command/src/commands/get.rs b/crates/nu-command/src/commands/get.rs index 60573aec8a..2d6fd3e926 100644 --- a/crates/nu-command/src/commands/get.rs +++ b/crates/nu-command/src/commands/get.rs @@ -1,5 +1,4 @@ use crate::prelude::*; -use crate::utils::arguments::arguments; use indexmap::set::IndexSet; use log::trace; use nu_engine::WholeStreamCommand; @@ -20,7 +19,7 @@ impl WholeStreamCommand for Command { fn signature(&self) -> Signature { Signature::build("get").rest( - SyntaxShape::Any, + SyntaxShape::ColumnPath, "optionally return additional data by path", ) } @@ -51,11 +50,9 @@ impl WholeStreamCommand for Command { pub fn get(args: CommandArgs) -> Result<ActionStream, ShellError> { let args = args.evaluate_once()?; - let mut rest: Vec<Value> = args.rest(0)?; + let column_paths: Vec<ColumnPath> = args.rest(0)?; let mut input = args.input; - let (column_paths, _) = arguments(&mut rest)?; - if column_paths.is_empty() { let vec = input.drain_vec(); diff --git a/crates/nu-command/src/commands/select.rs b/crates/nu-command/src/commands/select.rs index 5b5a9a8e7b..8a796f23cc 100644 --- a/crates/nu-command/src/commands/select.rs +++ b/crates/nu-command/src/commands/select.rs @@ -1,10 +1,9 @@ use crate::prelude::*; -use crate::utils::arguments::arguments; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; use nu_protocol::{ - PathMember, Primitive, Signature, SyntaxShape, TaggedDictBuilder, UnspannedPathMember, - UntaggedValue, Value, + ColumnPath, PathMember, Primitive, Signature, SyntaxShape, TaggedDictBuilder, + UnspannedPathMember, UntaggedValue, Value, }; use nu_value_ext::{as_string, get_data_by_column_path}; @@ -16,7 +15,10 @@ impl WholeStreamCommand for Command { } fn signature(&self) -> Signature { - Signature::build("select").rest(SyntaxShape::Any, "the columns to select from the table") + Signature::build("select").rest( + SyntaxShape::ColumnPath, + "the columns to select from the table", + ) } fn usage(&self) -> &str { @@ -46,9 +48,8 @@ impl WholeStreamCommand for Command { fn select(args: CommandArgs) -> Result<OutputStream, ShellError> { let name = args.call_info.name_tag.clone(); let args = args.evaluate_once()?; - let mut rest = args.rest(0)?; + let columns: Vec<ColumnPath> = args.rest(0)?; let input = args.input; - let (columns, _) = arguments(&mut rest)?; if columns.is_empty() { return Err(ShellError::labeled_error( diff --git a/crates/nu-command/src/commands/sort_by.rs b/crates/nu-command/src/commands/sort_by.rs index fbbbef1203..cc7eb1b23f 100644 --- a/crates/nu-command/src/commands/sort_by.rs +++ b/crates/nu-command/src/commands/sort_by.rs @@ -108,7 +108,7 @@ fn sort_by(args: CommandArgs) -> Result<OutputStream, ShellError> { let tag = args.call_info.name_tag.clone(); let mut args = args.evaluate_once()?; - let rest = args.rest(0)?; + let rest: Vec<Tagged<String>> = args.rest(0)?; let insensitive = args.has_flag("insensitive"); let reverse = args.has_flag("reverse"); let mut vec = args.input.drain_vec(); diff --git a/crates/nu-command/src/commands/str_/to_datetime.rs b/crates/nu-command/src/commands/str_/to_datetime.rs index ffd3bfd93d..6ce597b3db 100644 --- a/crates/nu-command/src/commands/str_/to_datetime.rs +++ b/crates/nu-command/src/commands/str_/to_datetime.rs @@ -1,5 +1,4 @@ use crate::prelude::*; -use crate::utils::arguments::arguments; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; use nu_protocol::{ @@ -127,16 +126,14 @@ impl WholeStreamCommand for SubCommand { struct DatetimeFormat(String); fn operate(args: CommandArgs) -> Result<ActionStream, ShellError> { - let (options, input) = args.extract(|params| { - let (column_paths, _) = arguments(&mut params.rest(0)?)?; - - Ok(Arguments { - timezone: params.get_flag("timezone")?, - offset: params.get_flag("offset")?, - format: params.get_flag("format")?, - column_paths, - }) - })?; + let args = args.evaluate_once()?; + let options = Arguments { + timezone: args.get_flag("timezone")?, + offset: args.get_flag("offset")?, + format: args.get_flag("format")?, + column_paths: args.rest(0)?, + }; + let input = args.input; // if zone-offset is specified, then zone will be neglected let zone_options = if let Some(Tagged { diff --git a/crates/nu-command/src/commands/str_/to_integer.rs b/crates/nu-command/src/commands/str_/to_integer.rs index a033eb0dc3..3bb96751a5 100644 --- a/crates/nu-command/src/commands/str_/to_integer.rs +++ b/crates/nu-command/src/commands/str_/to_integer.rs @@ -1,5 +1,4 @@ use crate::prelude::*; -use crate::utils::arguments::arguments; use nu_engine::WholeStreamCommand; use nu_errors::ShellError; use nu_protocol::ShellTypeName; @@ -25,7 +24,7 @@ impl WholeStreamCommand for SubCommand { Signature::build("str to-int") .named("radix", SyntaxShape::Number, "radix of integer", Some('r')) .rest( - SyntaxShape::Any, + SyntaxShape::ColumnPath, "optionally convert text into integer by column paths", ) } @@ -65,14 +64,12 @@ impl WholeStreamCommand for SubCommand { } fn operate(args: CommandArgs) -> Result<ActionStream, ShellError> { - let (options, input) = args.extract(|params| { - let (column_paths, _) = arguments(&mut params.rest(0)?)?; - - Ok(Arguments { - radix: params.get_flag("radix")?, - column_paths, - }) - })?; + let args = args.evaluate_once()?; + let options = Arguments { + radix: args.get_flag("radix")?, + column_paths: args.rest(0)?, + }; + let input = args.input; let radix = options.radix.as_ref().map(|r| r.item).unwrap_or(10); diff --git a/crates/nu-command/src/utils.rs b/crates/nu-command/src/utils.rs index e96612410d..ed6dc62fcf 100644 --- a/crates/nu-command/src/utils.rs +++ b/crates/nu-command/src/utils.rs @@ -1,3 +1,2 @@ -pub mod arguments; pub mod suggestions; pub mod test_bins; diff --git a/crates/nu-command/src/utils/arguments.rs b/crates/nu-command/src/utils/arguments.rs deleted file mode 100644 index e5bb2899f1..0000000000 --- a/crates/nu-command/src/utils/arguments.rs +++ /dev/null @@ -1,112 +0,0 @@ -use indexmap::IndexSet; -use nu_errors::ShellError; -use nu_protocol::{hir::CapturedBlock, ColumnPath, UntaggedValue, Value}; -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<Value>, -) -> Result<(Vec<ColumnPath>, Option<Box<CapturedBlock>>), ShellError> { - let last_argument = rest.pop(); - - let mut columns: IndexSet<_> = rest.iter().collect(); - let mut column_paths = vec![]; - - let mut default = None; - - for argument in columns.drain(..) { - 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 { - Some(Value { - value: UntaggedValue::Block(call), - .. - }) => default = Some(call), - 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<Vec<ColumnPath>, 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<dyn std::error::Error>> { - // 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<dyn std::error::Error>> { - // cmd name [type] - let arg1 = string("name"); - let arg2 = table(&[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<dyn std::error::Error>> { - // 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/empty.rs b/crates/nu-command/tests/commands/empty.rs index 0d92528578..f5ec14cf8e 100644 --- a/crates/nu-command/tests/commands/empty.rs +++ b/crates/nu-command/tests/commands/empty.rs @@ -32,7 +32,7 @@ fn sets_block_run_value_for_an_empty_column() { [ Jason, Gedge, 10/11/2013, 1 ] [ Yehuda, Katz, 10/11/2013, '' ] ] - | empty? likes { 1 } + | empty? likes -b { 1 } | get likes | math sum "# @@ -52,7 +52,7 @@ fn sets_block_run_value_for_many_empty_columns() { [ 1, "" ] [ 1, (wrap) ] ] - | empty? boost check { 1 } + | empty? boost check -b { 1 } | get boost check | math sum "# @@ -73,8 +73,8 @@ fn passing_a_block_will_set_contents_on_empty_cells_and_leave_non_empty_ones_unt [ Arepas, "", "" ] [ Jorge, 30, 3000 ] ] - | empty? LVL { 9 } - | empty? HP { + | empty? LVL -b { 9 } + | empty? HP -b { $it.LVL * 1000 } | math sum