ColumnPath creation flexibility. (#2674)

Cargo.lock

@@ -3140,6 +3140,7 @@ dependencies = [
  "nu-errors",
  "nu-protocol",
  "nu-source",
+ "nu-test-support",
  "num-traits 0.2.12",
 ]
 

crates/nu-data/src/base.rs

@@ -191,7 +191,7 @@ mod tests {
 
     #[test]
     fn gets_matching_field_from_nested_rows_inside_a_row() -> Result<(), ShellError> {
-        let field_path = column_path(&[string("package"), string("version")]);
+        let field_path = column_path("package.version");
 
         let (version, tag) = string("0.4.0").into_parts();
 
@@ -217,7 +217,7 @@ mod tests {
     #[test]
     fn gets_first_matching_field_from_rows_with_same_field_inside_a_table() -> Result<(), ShellError>
     {
-        let field_path = column_path(&[string("package"), string("authors"), string("name")]);
+        let field_path = column_path("package.authors.name");
 
         let (_, tag) = string("Andrés N. Robalino").into_parts();
 
@@ -250,7 +250,7 @@ mod tests {
 
     #[test]
     fn column_path_that_contains_just_a_number_gets_a_row_from_a_table() -> Result<(), ShellError> {
-        let field_path = column_path(&[string("package"), string("authors"), int(0)]);
+        let field_path = column_path("package.authors.0");
 
         let (_, tag) = string("Andrés N. Robalino").into_parts();
 
@@ -281,7 +281,7 @@ mod tests {
 
     #[test]
     fn column_path_that_contains_just_a_number_gets_a_row_from_a_row() -> Result<(), ShellError> {
-        let field_path = column_path(&[string("package"), string("authors"), string("0")]);
+        let field_path = column_path(r#"package.authors."0""#);
 
         let (_, tag) = string("Andrés N. Robalino").into_parts();
 
@@ -312,7 +312,7 @@ mod tests {
 
     #[test]
     fn replaces_matching_field_from_a_row() -> Result<(), ShellError> {
-        let field_path = column_path(&[string("amigos")]);
+        let field_path = column_path("amigos");
 
         let sample = UntaggedValue::row(indexmap! {
             "amigos".into() => table(&[
@@ -336,11 +336,7 @@ mod tests {
 
     #[test]
     fn replaces_matching_field_from_nested_rows_inside_a_row() -> Result<(), ShellError> {
-        let field_path = column_path(&[
-            string("package"),
-            string("authors"),
-            string("los.3.caballeros"),
-        ]);
+        let field_path = column_path(r#"package.authors."los.3.caballeros""#);
 
         let sample = UntaggedValue::row(indexmap! {
             "package".into() => row(indexmap! {
@@ -381,11 +377,7 @@ mod tests {
     }
     #[test]
     fn replaces_matching_field_from_rows_inside_a_table() -> Result<(), ShellError> {
-        let field_path = column_path(&[
-            string("shell_policy"),
-            string("releases"),
-            string("nu.version.arepa"),
-        ]);
+        let field_path = column_path(r#"shell_policy.releases."nu.version.arepa""#);
 
         let sample = UntaggedValue::row(indexmap! {
             "shell_policy".into() => row(indexmap! {

crates/nu-parser/src/parse.rs

@@ -18,7 +18,9 @@ use crate::signature::SignatureRegistry;
 use bigdecimal::BigDecimal;
 
 /// Parses a simple column path, one without a variable (implied or explicit) at the head
-fn parse_simple_column_path(lite_arg: &Spanned<String>) -> (SpannedExpression, Option<ParseError>) {
+pub fn parse_simple_column_path(
+    lite_arg: &Spanned<String>,
+) -> (SpannedExpression, Option<ParseError>) {
     let mut delimiter = '.';
     let mut inside_delimiter = false;
     let mut output = vec![];

crates/nu-protocol/src/value.rs

@@ -16,13 +16,13 @@ use crate::value::dict::Dictionary;
 use crate::value::iter::{RowValueIter, TableValueIter};
 use crate::value::primitive::Primitive;
 use crate::value::range::{Range, RangeInclusion};
-use crate::{ColumnPath, PathMember};
+use crate::ColumnPath;
 use bigdecimal::BigDecimal;
 use bigdecimal::FromPrimitive;
 use chrono::{DateTime, Utc};
 use indexmap::IndexMap;
 use nu_errors::ShellError;
-use nu_source::{AnchorLocation, HasSpan, Span, Spanned, Tag};
+use nu_source::{AnchorLocation, HasSpan, Span, Spanned, SpannedItem, Tag};
 use num_bigint::BigInt;
 use num_traits::ToPrimitive;
 use serde::{Deserialize, Serialize};
@@ -169,10 +169,10 @@ impl UntaggedValue {
     }
 
     /// Helper for creating column-path values
-    pub fn column_path(s: Vec<impl Into<PathMember>>) -> UntaggedValue {
-        UntaggedValue::Primitive(Primitive::ColumnPath(ColumnPath::new(
-            s.into_iter().map(|p| p.into()).collect(),
-        )))
+    pub fn column_path(s: &str) -> UntaggedValue {
+        let s = s.to_string().spanned_unknown();
+
+        UntaggedValue::Primitive(Primitive::ColumnPath(ColumnPath::build(&s)))
     }
 
     /// Helper for creating integer values

crates/nu-protocol/src/value/column_path.rs

@@ -1,9 +1,15 @@
 use derive_new::new;
 use getset::Getters;
-use nu_source::{b, span_for_spanned_list, DebugDocBuilder, HasFallibleSpan, PrettyDebug, Span};
+use nu_source::{
+    b, span_for_spanned_list, DebugDocBuilder, HasFallibleSpan, PrettyDebug, Span, Spanned,
+    SpannedItem,
+};
 use num_bigint::BigInt;
 use serde::{Deserialize, Serialize};
 
+use crate::hir::{Expression, Literal, Member, SpannedExpression};
+use nu_errors::ParseError;
+
 /// A PathMember that has yet to be spanned so that it can be used in later processing
 #[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
 pub enum UnspannedPathMember {
@@ -65,6 +71,23 @@ impl ColumnPath {
     pub fn last(&self) -> Option<&PathMember> {
         self.iter().last()
     }
+
+    pub fn build(text: &Spanned<String>) -> ColumnPath {
+        if let (
+            SpannedExpression {
+                expr: Expression::Literal(Literal::ColumnPath(path)),
+                span: _,
+            },
+            _,
+        ) = parse(&text)
+        {
+            ColumnPath {
+                members: path.iter().map(|member| member.to_path_member()).collect(),
+            }
+        } else {
+            ColumnPath { members: vec![] }
+        }
+    }
 }
 
 impl PrettyDebug for ColumnPath {
@@ -111,3 +134,71 @@ impl PathMember {
         }
     }
 }
+
+fn parse(raw_column_path: &Spanned<String>) -> (SpannedExpression, Option<ParseError>) {
+    let mut delimiter = '.';
+    let mut inside_delimiter = false;
+    let mut output = vec![];
+    let mut current_part = String::new();
+    let mut start_index = 0;
+    let mut last_index = 0;
+
+    for (idx, c) in raw_column_path.item.char_indices() {
+        last_index = idx;
+        if inside_delimiter {
+            if c == delimiter {
+                inside_delimiter = false;
+            }
+        } else if c == '\'' || c == '"' || c == '`' {
+            inside_delimiter = true;
+            delimiter = c;
+        } else if c == '.' {
+            let part_span = Span::new(
+                raw_column_path.span.start() + start_index,
+                raw_column_path.span.start() + idx,
+            );
+
+            if let Ok(row_number) = current_part.parse::<u64>() {
+                output.push(Member::Int(BigInt::from(row_number), part_span));
+            } else {
+                let trimmed = trim_quotes(&current_part);
+                output.push(Member::Bare(trimmed.clone().spanned(part_span)));
+            }
+            current_part.clear();
+            // Note: I believe this is safe because of the delimiter we're using, but if we get fancy with
+            // unicode we'll need to change this
+            start_index = idx + '.'.len_utf8();
+            continue;
+        }
+        current_part.push(c);
+    }
+
+    if !current_part.is_empty() {
+        let part_span = Span::new(
+            raw_column_path.span.start() + start_index,
+            raw_column_path.span.start() + last_index + 1,
+        );
+        if let Ok(row_number) = current_part.parse::<u64>() {
+            output.push(Member::Int(BigInt::from(row_number), part_span));
+        } else {
+            let current_part = trim_quotes(&current_part);
+            output.push(Member::Bare(current_part.spanned(part_span)));
+        }
+    }
+
+    (
+        SpannedExpression::new(Expression::simple_column_path(output), raw_column_path.span),
+        None,
+    )
+}
+
+fn trim_quotes(input: &str) -> String {
+    let mut chars = input.chars();
+
+    match (chars.next(), chars.next_back()) {
+        (Some('\''), Some('\'')) => chars.collect(),
+        (Some('"'), Some('"')) => chars.collect(),
+        (Some('`'), Some('`')) => chars.collect(),
+        _ => input.to_string(),
+    }
+}

crates/nu-test-support/src/value.rs

@@ -2,8 +2,7 @@ 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 nu_source::{Span, SpannedItem, Tagged, TaggedItem};
 use num_bigint::BigInt;
 
 pub fn int(s: impl Into<BigInt>) -> Value {
@@ -43,8 +42,9 @@ pub fn date(input: impl Into<String>) -> Value {
     .into_untagged_value()
 }
 
-pub fn column_path(paths: &[Value]) -> Result<Tagged<ColumnPath>, ShellError> {
-    as_column_path(&table(paths))
+pub fn column_path(paths: &str) -> Result<Tagged<ColumnPath>, ShellError> {
+    let paths = paths.to_string().spanned_unknown();
+    Ok(ColumnPath::build(&paths).tagged_unknown())
 }
 
 pub fn error_callback(

crates/nu-value-ext/Cargo.toml

@@ -17,3 +17,6 @@ nu-source = {path = "../nu-source", version = "0.21.0"}
 indexmap = {version = "1.6.0", features = ["serde-1"]}
 itertools = "0.9.0"
 num-traits = "0.2.12"
+
+[dev-dependencies]
+nu-test-support = {path = "../nu-test-support", version = "0.21.0"}

crates/nu-value-ext/src/lib.rs

@@ -1,3 +1,6 @@
+#[cfg(test)]
+mod tests;
+
 use indexmap::indexmap;
 use indexmap::set::IndexSet;
 use itertools::Itertools;
@@ -639,7 +642,9 @@ pub fn as_column_path(value: &Value) -> Result<Tagged<ColumnPath>, ShellError> {
         }
 
         UntaggedValue::Primitive(Primitive::String(s)) => {
-            Ok(ColumnPath::new(vec![PathMember::string(s, &value.tag.span)]).tagged(&value.tag))
+            let s = s.to_string().spanned(value.tag.span);
+
+            Ok(ColumnPath::build(&s).tagged(&value.tag))
         }
 
         UntaggedValue::Primitive(Primitive::ColumnPath(path)) => {

crates/nu-value-ext/src/tests.rs

@@ -0,0 +1,137 @@
+use super::*;
+use nu_test_support::value::*;
+
+use indexmap::indexmap;
+
+#[test]
+fn forgiving_insertion_test_1() {
+    let field_path = column_path("crate.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("things.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("color_config.arepa_color").unwrap();
+    let pizza_path = column_path("things.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")
+    );
+}
+
+#[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()])
+    );
+}