Soft rest arguments column path cohersions. (#3016)

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),

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 {
+pub struct Arguments {
-    rest: Vec<ColumnPath>,
+    rest: Vec<Value>,
 }
 
 #[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<OutputStream, ShellError> {
-    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 {})?)
-    }
-}

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,
+    PathMember, Primitive, ReturnSuccess, Signature, SyntaxShape, TaggedDictBuilder,
-    TaggedDictBuilder, UnspannedPathMember, UntaggedValue, Value,
+    UnspannedPathMember, UntaggedValue, Value,
 };
 use nu_value_ext::{as_string, get_data_by_column_path};
+
 #[derive(Deserialize)]
 struct Arguments {
     rest: Vec<Value>,
@@ -51,7 +52,7 @@ impl WholeStreamCommand for Command {
 async fn select(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let name = args.call_info.name_tag.clone();
     let (Arguments { mut rest }, mut input) = args.process().await?;
-    let (columns, _): (Vec<ColumnPath>, Option<Box<CapturedBlock>>) = 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<OutputStream, ShellError> {
         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()),
+                    Some(row) => out.insert_untagged(new_key, row.get_data(k).borrow().clone()),
-                    None => out.insert_untagged(k, nothing.clone()),
+                    None => out.insert_untagged(new_key, nothing.clone()),
                 },
-                None => out.insert_untagged(k, nothing.clone()),
+                None => out.insert_untagged(new_key, nothing.clone()),
             }
         }
 

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<Value>,
 ) -> Result<(Vec<ColumnPath>, Option<Box<CapturedBlock>>), 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()?;
+        match &argument.value {
-
+            UntaggedValue::Table(values) => {
-        column_paths.push(path);
+                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) => {
+        Some(other) => match &other.value {
-            let Tagged { item: path, .. } = other.as_column_path()?;
+            UntaggedValue::Table(values) => {
-
+                column_paths.extend(collect_as_column_paths(&values)?);
-            column_paths.push(path);
+            }
-        }
+            _ => {
+                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(&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<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/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")

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"
+                | where nu_releases_version > "0.8"
-                | get "nu.releases.version"
+                | get nu_releases_version
             "#
         ));
 

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<Tagged<ColumnPath>, ShellError> {
     match &value.value {
-        UntaggedValue::Table(table) => {
-            let mut out: Vec<PathMember> = 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);
 

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()])
+    );
+}