Fix #3582 (#3583)

* Fix #3582

* Fix empty?

* Fix parsing types

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(
+        Signature::build("empty?")
-            SyntaxShape::Any,
+            .rest(
-            "the names of the columns to check emptiness. Pass an optional block to replace if empty",
+                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 block: Option<CapturedBlock> = args.get_flag("block")?;
-    let (columns, default_block): (Vec<ColumnPath>, Option<Box<CapturedBlock>>) =
+    let columns: Vec<ColumnPath> = args.rest(0)?;
-        arguments(&mut rest)?;
+
     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> {

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();
 

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,
+    ColumnPath, PathMember, Primitive, Signature, SyntaxShape, TaggedDictBuilder,
-    UntaggedValue, Value,
+    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(

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();

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 args = args.evaluate_once()?;
-        let (column_paths, _) = arguments(&mut params.rest(0)?)?;
+    let options = Arguments {
-
+        timezone: args.get_flag("timezone")?,
-        Ok(Arguments {
+        offset: args.get_flag("offset")?,
-            timezone: params.get_flag("timezone")?,
+        format: args.get_flag("format")?,
-            offset: params.get_flag("offset")?,
+        column_paths: args.rest(0)?,
-            format: params.get_flag("format")?,
+    };
-            column_paths,
+    let input = args.input;
-        })
-    })?;
 
     // if zone-offset is specified, then zone will be neglected
     let zone_options = if let Some(Tagged {

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 args = args.evaluate_once()?;
-        let (column_paths, _) = arguments(&mut params.rest(0)?)?;
+    let options = Arguments {
-
+        radix: args.get_flag("radix")?,
-        Ok(Arguments {
+        column_paths: args.rest(0)?,
-            radix: params.get_flag("radix")?,
+    };
-            column_paths,
+    let input = args.input;
-        })
-    })?;
 
     let radix = options.radix.as_ref().map(|r| r.item).unwrap_or(10);
 

crates/nu-command/src/utils.rs

@@ -1,3 +1,2 @@
-pub mod arguments;
 pub mod suggestions;
 pub mod test_bins;

crates/nu-command/src/utils/arguments.rs

@@ -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(())
-    }
-}

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? LVL -b { 9 }
-            | empty? HP {
+            | empty? HP -b {
                 $it.LVL * 1000
               }
             | math sum