From 6ea8e42331f25df9064c2e9727afded33dab33be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Fri, 1 Nov 2019 16:19:46 -0500 Subject: [PATCH 1/3] Move column paths to support broader value types. --- src/commands/get.rs | 10 +- src/data/base.rs | 126 +++++++----------- src/parser/deserializer.rs | 6 +- .../hir/syntax_shape/expression/number.rs | 19 ++- .../syntax_shape/expression/variable_path.rs | 10 ++ src/parser/hir/tokens_iterator/tests.rs | 16 +++ src/parser/parse/token_tree.rs | 10 ++ src/plugins/edit.rs | 2 +- src/plugins/inc.rs | 20 +-- src/plugins/insert.rs | 13 +- src/plugins/str.rs | 35 +++-- src/utils.rs | 4 +- tests/command_get_tests.rs | 53 +++++--- 13 files changed, 201 insertions(+), 123 deletions(-) create mode 100644 src/parser/hir/tokens_iterator/tests.rs diff --git a/src/commands/get.rs b/src/commands/get.rs index 0176b215d..f5db529c6 100644 --- a/src/commands/get.rs +++ b/src/commands/get.rs @@ -44,7 +44,7 @@ impl WholeStreamCommand for Get { } } -pub type ColumnPath = Vec>; +pub type ColumnPath = Vec>; pub fn get_column_path( path: &ColumnPath, @@ -67,7 +67,13 @@ pub fn get_column_path( return ShellError::labeled_error_with_secondary( "Row not found", - format!("There isn't a row indexed at '{}'", **column_path_tried), + format!( + "There isn't a row indexed at '{}'", + match &*column_path_tried { + Value::Primitive(primitive) => primitive.format(None), + _ => String::from(""), + } + ), column_path_tried.tag(), format!("The table only has {} rows (0..{})", total, total - 1), end_tag, diff --git a/src/data/base.rs b/src/data/base.rs index 17691e24b..094d425ee 100644 --- a/src/data/base.rs +++ b/src/data/base.rs @@ -409,25 +409,17 @@ impl Tagged { ValueDebug { value: self } } - pub fn as_column_path(&self) -> Result>>, ShellError> { - let mut out: Vec> = vec![]; - + pub fn as_column_path(&self) -> Result>>, ShellError> { match &self.item { - Value::Table(table) => { - for item in table { - out.push(item.as_string()?.tagged(&item.tag)); - } - } - - other => { - return Err(ShellError::type_error( - "column name", - other.type_name().tagged(&self.tag), - )) + Value::Primitive(Primitive::String(s)) => { + Ok(vec![Value::string(s).tagged(&self.tag)].tagged(&self.tag)) } + Value::Table(table) => Ok(table.to_vec().tagged(&self.tag)), + other => Err(ShellError::type_error( + "column name", + other.type_name().tagged(&self.tag), + )), } - - Ok(out.tagged(&self.tag)) } pub(crate) fn as_string(&self) -> Result { @@ -528,30 +520,32 @@ impl Value { pub fn get_data_by_column_path( &self, tag: Tag, - path: &Vec>, - callback: Box)) -> ShellError>, + path: &Vec>, + callback: Box)) -> ShellError>, ) -> Result>, ShellError> { let mut current = self; for p in path { - // note: - // This will eventually be refactored once we are able - // to parse correctly column_paths and get them deserialized - // to values for us. - let value = match p.item().parse::() { - Ok(number) => match current { - Value::Table(_) => current.get_data_by_index(number), - Value::Row(_) => current.get_data_by_key(p), - _ => None, - }, - Err(_) => match self { - Value::Table(_) | Value::Row(_) => current.get_data_by_key(p), - _ => None, - }, - }; // end + let value = match p.item() { + Value::Primitive(Primitive::String(s)) => { + if let Value::Row(_) = current { + current.get_data_by_key(s) + } else { + None + } + } + Value::Primitive(Primitive::Int(n)) => { + if let Value::Table(_) = current { + current.get_data_by_index(n.to_usize().unwrap()) + } else { + None + } + } + _ => None, + }; match value { Some(v) => current = v, - None => return Err(callback((¤t.clone(), &p.clone()))), + None => return Err(callback((current.clone(), p.clone()))), } } @@ -614,9 +608,21 @@ impl Value { pub fn insert_data_at_column_path( &self, tag: Tag, - split_path: &Vec>, + split_path: &Vec>, new_value: Value, ) -> Option> { + let split_path = split_path + .into_iter() + .map(|p| match p { + Tagged { + item: Value::Primitive(Primitive::String(s)), + tag, + } => Ok(s.clone().tagged(tag)), + o => Err(o), + }) + .filter_map(Result::ok) + .collect::>>(); + let mut new_obj = self.clone(); if let Value::Row(ref mut o) = new_obj { @@ -665,14 +671,14 @@ impl Value { pub fn replace_data_at_column_path( &self, tag: Tag, - split_path: &Vec>, + split_path: &Vec>, replaced_value: Value, ) -> Option> { let mut new_obj = self.clone(); let mut current = &mut new_obj; for idx in 0..split_path.len() { - match current.get_mut_data_by_key(&split_path[idx].item) { + match current.get_mut_data_by_key(&split_path[idx].as_string().unwrap()) { Some(next) => { if idx == (split_path.len() - 1) { *next = replaced_value.tagged(&tag); @@ -943,6 +949,10 @@ mod tests { Value::string(input.into()).tagged_unknown() } + fn number(n: i64) -> Tagged { + Value::number(n).tagged_unknown() + } + fn row(entries: IndexMap>) -> Tagged { Value::row(entries).tagged_unknown() } @@ -951,19 +961,12 @@ mod tests { Value::table(list).tagged_unknown() } - fn error_callback() -> impl FnOnce((&Value, &Tagged)) -> ShellError { + fn error_callback() -> impl FnOnce((Value, Tagged)) -> ShellError { move |(_obj_source, _column_path_tried)| ShellError::unimplemented("will never be called.") } - fn column_path(paths: &Vec>) -> Tagged>> { - table( - &paths - .iter() - .map(|p| string(p.as_string().unwrap())) - .collect(), - ) - .as_column_path() - .unwrap() + fn column_path(paths: &Vec>) -> Vec> { + table(paths).as_column_path().unwrap().item } #[test] @@ -1005,36 +1008,9 @@ mod tests { ) } - #[test] - fn gets_first_matching_field_from_rows_with_same_field_inside_a_table() { - let field_path = column_path(&vec![string("package"), string("authors"), string("name")]); - - let (name, tag) = string("Andrés N. Robalino").into_parts(); - - let value = Value::row(indexmap! { - "package".into() => row(indexmap! { - "name".into() => string("nu"), - "version".into() => string("0.4.0"), - "authors".into() => table(&vec![ - row(indexmap!{"name".into() => string("Andrés N. Robalino")}), - row(indexmap!{"name".into() => string("Jonathan Turner")}), - row(indexmap!{"name".into() => string("Yehuda Katz")}) - ]) - }) - }); - - assert_eq!( - **value - .get_data_by_column_path(tag, &field_path, Box::new(error_callback())) - .unwrap() - .unwrap(), - name - ) - } - #[test] fn column_path_that_contains_just_a_number_gets_a_row_from_a_table() { - let field_path = column_path(&vec![string("package"), string("authors"), string("0")]); + let field_path = column_path(&vec![string("package"), string("authors"), number(0)]); let (_, tag) = string("Andrés N. Robalino").into_parts(); diff --git a/src/parser/deserializer.rs b/src/parser/deserializer.rs index 4b8bf913d..c9436ab29 100644 --- a/src/parser/deserializer.rs +++ b/src/parser/deserializer.rs @@ -61,7 +61,7 @@ impl<'de> ConfigDeserializer<'de> { pub fn top(&mut self) -> &DeserializerItem { let value = self.stack.last(); trace!("inspecting top value :: {:?}", value); - value.expect("Can't get top elemant of an empty stack") + value.expect("Can't get top element of an empty stack") } pub fn pop(&mut self) -> DeserializerItem { @@ -486,8 +486,8 @@ mod tests { // is unspecified and change is likely. // This test makes sure that such change is detected // by this test failing, and not things silently breaking. - // Specifically, we rely on this behaviour further above - // in the file to special case Tagged parsing. + // Specifically, we rely on this behavior further above + // in the file for the Tagged special case parsing. let tuple = type_name::<()>(); let tagged_tuple = type_name::>(); let tagged_value = type_name::>(); diff --git a/src/parser/hir/syntax_shape/expression/number.rs b/src/parser/hir/syntax_shape/expression/number.rs index 6c599cc02..9a7a6227d 100644 --- a/src/parser/hir/syntax_shape/expression/number.rs +++ b/src/parser/hir/syntax_shape/expression/number.rs @@ -1,7 +1,7 @@ use crate::parser::hir::syntax_shape::{ expand_atom, parse_single_node, ExpandContext, ExpandExpression, ExpansionRule, - FallibleColorSyntax, FlatShape, ParseError, -}; + FallibleColorSyntax, FlatShape, TestSyntax, ParseError}; +use crate::parser::hir::tokens_iterator::Peeked; use crate::parser::{ hir, hir::{RawNumber, TokensIterator}, @@ -212,3 +212,18 @@ impl FallibleColorSyntax for IntShape { Ok(()) } } + +impl TestSyntax for NumberShape { + fn test<'a, 'b>( + &self, + token_nodes: &'b mut TokensIterator<'a>, + _context: &ExpandContext, + ) -> Option> { + let peeked = token_nodes.peek_any(); + + match peeked.node { + Some(token) if token.is_number() => Some(peeked), + _ => None, + } + } +} diff --git a/src/parser/hir/syntax_shape/expression/variable_path.rs b/src/parser/hir/syntax_shape/expression/variable_path.rs index 1a91e132c..f6b4c1931 100644 --- a/src/parser/hir/syntax_shape/expression/variable_path.rs +++ b/src/parser/hir/syntax_shape/expression/variable_path.rs @@ -906,6 +906,16 @@ impl ExpandSyntax for MemberShape { return Ok(Member::Bare(node.span())); } + /* KATZ */ + /* let number = NumberShape.test(token_nodes, context); + + if let Some(peeked) = number { + let node = peeked.not_eof("column")?.commit(); + let (n, span) = node.as_number().unwrap(); + + return Ok(Member::Number(n, span)) + }*/ + let string = StringShape.test(token_nodes, context); if let Some(peeked) = string { diff --git a/src/parser/hir/tokens_iterator/tests.rs b/src/parser/hir/tokens_iterator/tests.rs new file mode 100644 index 000000000..23f888978 --- /dev/null +++ b/src/parser/hir/tokens_iterator/tests.rs @@ -0,0 +1,16 @@ +use crate::parser::hir::TokensIterator; +use crate::parser::parse::token_tree_builder::TokenTreeBuilder as b; +use crate::Span; + +#[test] +fn supplies_tokens() { + let tokens = b::token_list(vec![b::var("it"), b::op("."), b::bare("cpu")]); + let (tokens, _) = b::build(tokens); + + let tokens = tokens.expect_list(); + let mut iterator = TokensIterator::all(tokens, Span::unknown()); + + iterator.next().unwrap().expect_var(); + iterator.next().unwrap().expect_dot(); + iterator.next().unwrap().expect_bare(); +} diff --git a/src/parser/parse/token_tree.rs b/src/parser/parse/token_tree.rs index 75228133d..137b22be7 100644 --- a/src/parser/parse/token_tree.rs +++ b/src/parser/parse/token_tree.rs @@ -171,6 +171,16 @@ impl TokenNode { } } + pub fn is_number(&self) -> bool { + match self { + TokenNode::Token(Spanned { + item: RawToken::Number(_), + .. + }) => true, + _ => false, + } + } + pub fn as_string(&self) -> Option<(Span, Span)> { match self { TokenNode::Token(Spanned { diff --git a/src/plugins/edit.rs b/src/plugins/edit.rs index 78cb32cef..fb0ac48ed 100644 --- a/src/plugins/edit.rs +++ b/src/plugins/edit.rs @@ -3,7 +3,7 @@ use nu::{ Tagged, Value, }; -pub type ColumnPath = Tagged>>; +pub type ColumnPath = Tagged>>; struct Edit { field: Option, diff --git a/src/plugins/inc.rs b/src/plugins/inc.rs index fb3836dfd..b7b24025e 100644 --- a/src/plugins/inc.rs +++ b/src/plugins/inc.rs @@ -14,7 +14,7 @@ pub enum SemVerAction { Patch, } -pub type ColumnPath = Vec>; +pub type ColumnPath = Tagged>>; struct Inc { field: Option, @@ -100,7 +100,7 @@ impl Inc { let replace_for = value.item.get_data_by_column_path( value.tag(), - &f, + f, Box::new(move |(obj_source, column_path_tried)| { match did_you_mean(&obj_source, &column_path_tried) { Some(suggestions) => { @@ -191,7 +191,7 @@ impl Plugin for Inc { item: Value::Table(_), .. } => { - self.field = Some(table.as_column_path()?.item().to_vec()); + self.field = Some(table.as_column_path()?); } value => return Err(ShellError::type_error("table", value.tagged_type_name())), } @@ -229,8 +229,8 @@ mod tests { use super::{Inc, SemVerAction}; use indexmap::IndexMap; use nu::{ - CallInfo, EvaluatedArgs, Plugin, ReturnSuccess, Tag, Tagged, TaggedDictBuilder, TaggedItem, - Value, + CallInfo, EvaluatedArgs, Plugin, Primitive, ReturnSuccess, Tag, Tagged, TaggedDictBuilder, + TaggedItem, Value, }; struct CallStub { @@ -344,9 +344,13 @@ mod tests { .is_ok()); assert_eq!( - plugin - .field - .map(|f| f.iter().map(|f| f.item.clone()).collect()), + plugin.field.map(|f| f + .iter() + .map(|f| match &f.item { + Value::Primitive(Primitive::String(s)) => s.clone(), + _ => panic!(""), + }) + .collect()), Some(vec!["package".to_string(), "version".to_string()]) ); } diff --git a/src/plugins/insert.rs b/src/plugins/insert.rs index 9041abc36..cfe3a27b5 100644 --- a/src/plugins/insert.rs +++ b/src/plugins/insert.rs @@ -4,7 +4,7 @@ use nu::{ Tagged, TaggedItem, Value, }; -pub type ColumnPath = Vec>; +pub type ColumnPath = Vec>; struct Insert { field: Option, @@ -22,14 +22,21 @@ impl Insert { let value_tag = value.tag(); match (value.item, self.value.clone()) { (obj @ Value::Row(_), Some(v)) => match &self.field { - Some(f) => match obj.insert_data_at_column_path(value_tag.clone(), &f, v) { + Some(f) => match obj.insert_data_at_column_path(value_tag.clone(), f, v) { Some(v) => return Ok(v), None => { return Err(ShellError::labeled_error( format!( "add could not find place to insert field {:?} {}", obj, - f.iter().map(|i| &i.item).join(".") + f.iter() + .map(|i| { + match &i.item { + Value::Primitive(primitive) => primitive.format(None), + _ => String::from(""), + } + }) + .join(".") ), "column name", &value_tag, diff --git a/src/plugins/str.rs b/src/plugins/str.rs index 7fb694d3a..552602acc 100644 --- a/src/plugins/str.rs +++ b/src/plugins/str.rs @@ -12,7 +12,7 @@ enum Action { Substring(usize, usize), } -pub type ColumnPath = Vec>; +pub type ColumnPath = Tagged>>; struct Str { field: Option, @@ -132,7 +132,7 @@ impl Str { let replace_for = value.item.get_data_by_column_path( value.tag(), - &f, + f, Box::new(move |(obj_source, column_path_tried)| { match did_you_mean(&obj_source, &column_path_tried) { Some(suggestions) => { @@ -169,7 +169,7 @@ impl Str { match value.item.replace_data_at_column_path( value.tag(), - &f, + f, replacement.item.clone(), ) { Some(v) => return Ok(v), @@ -246,17 +246,22 @@ impl Plugin for Str { if let Some(possible_field) = args.nth(0) { match possible_field { - Tagged { - item: Value::Primitive(Primitive::String(s)), - tag, - } => { - self.for_field(vec![s.clone().tagged(tag)]); - } + string @ Tagged { + item: Value::Primitive(Primitive::String(_)), + .. + } => match self.action { + Some(Action::Downcase) + | Some(Action::Upcase) + | Some(Action::ToInteger) + | None => { + self.for_field(string.as_column_path()?); + } + }, table @ Tagged { item: Value::Table(_), .. } => { - self.field = Some(table.as_column_path()?.item); + self.field = Some(table.as_column_path()?); } _ => { return Err(ShellError::labeled_error( @@ -419,9 +424,13 @@ mod tests { .is_ok()); assert_eq!( - plugin - .field - .map(|f| f.into_iter().map(|f| f.item).collect()), + plugin.field.map(|f| f + .iter() + .map(|f| match &f.item { + Value::Primitive(Primitive::String(s)) => s.clone(), + _ => panic!(""), + }) + .collect()), Some(vec!["package".to_string(), "description".to_string()]) ) } diff --git a/src/utils.rs b/src/utils.rs index 9822b7627..cef4aad19 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -7,8 +7,10 @@ use std::path::{Component, Path, PathBuf}; pub fn did_you_mean( obj_source: &Value, - field_tried: &Tagged, + field_tried: &Tagged, ) -> Option> { + let field_tried = field_tried.as_string().unwrap(); + let possibilities = obj_source.data_descriptors(); let mut possible_matches: Vec<_> = possibilities diff --git a/tests/command_get_tests.rs b/tests/command_get_tests.rs index 09348678b..386b1795e 100644 --- a/tests/command_get_tests.rs +++ b/tests/command_get_tests.rs @@ -27,7 +27,7 @@ fn get() { } #[test] -fn fetches_by_index_from_a_given_table() { +fn fetches_by_index() { Playground::setup("get_test_2", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( "sample.toml", @@ -53,14 +53,13 @@ fn fetches_by_index_from_a_given_table() { }) } #[test] -fn supports_fetching_rows_from_tables_using_columns_named_as_numbers() { +fn fetches_by_column_path() { Playground::setup("get_test_3", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( "sample.toml", r#" [package] - 0 = "nu" - 1 = "0.4.1" + name = "nu" "#, )]); @@ -68,25 +67,23 @@ fn supports_fetching_rows_from_tables_using_columns_named_as_numbers() { cwd: dirs.test(), h::pipeline( r#" open sample.toml - | get package.1 + | get package.name | echo $it "# )); - assert_eq!(actual, "0.4.1"); + assert_eq!(actual, "nu"); }) } #[test] -fn can_fetch_tables_or_rows_using_numbers_in_column_path() { +fn column_paths_are_either_double_quoted_or_regular_unquoted_words_separated_by_dot() { Playground::setup("get_test_4", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( "sample.toml", r#" [package] - 0 = "nu" - 1 = "0.4.1" - 2 = ["Yehuda Katz ", "Jonathan Turner ", "Andrés N. Robalino "] + 9999 = ["Yehuda Katz ", "Jonathan Turner ", "Andrés N. Robalino "] description = "When arepas shells are tasty and fun." "#, )]); @@ -95,17 +92,18 @@ fn can_fetch_tables_or_rows_using_numbers_in_column_path() { cwd: dirs.test(), h::pipeline( r#" open sample.toml - | get package.2.1 + | get package."9999" + | count | echo $it "# )); - assert_eq!(actual, "Jonathan Turner "); + assert_eq!(actual, "3"); }) } #[test] -fn fetches_more_than_one_column_member_path() { +fn fetches_more_than_one_column_path() { Playground::setup("get_test_5", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( "sample.toml", @@ -161,9 +159,34 @@ fn errors_fetching_by_column_not_present() { assert!(actual.contains("did you mean 'taconushell'?")); }) } + #[test] -fn errors_fetching_by_index_out_of_bounds_from_table() { +fn errors_fetching_by_column_using_a_number() { Playground::setup("get_test_7", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "sample.toml", + r#" + [spanish_lesson] + 0 = "can only be fetched with 0 double quoted." + "#, + )]); + + let actual = nu_error!( + cwd: dirs.test(), h::pipeline( + r#" + open sample.toml + | get spanish_lesson.0 + "# + )); + + assert!(actual.contains("No rows available")); + assert!(actual.contains("Tried getting a row indexed at '0'")); + assert!(actual.contains(r#"Not a table. Perhaps you meant to get the column "0" instead?"#)) + }) +} +#[test] +fn errors_fetching_by_index_out_of_bounds() { + Playground::setup("get_test_8", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( "sample.toml", r#" @@ -188,7 +211,7 @@ fn errors_fetching_by_index_out_of_bounds_from_table() { #[test] fn requires_at_least_one_column_member_path() { - Playground::setup("get_test_8", |dirs, sandbox| { + Playground::setup("get_test_9", |dirs, sandbox| { sandbox.with_files(vec![EmptyFile("andres.txt")]); let actual = nu_error!( From d7b768ee9f8a34f7142a4c71b9029d6f20e5faf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Sun, 3 Nov 2019 05:19:09 -0500 Subject: [PATCH 2/3] Fallback internally to String primitives until Member int serialization lands. --- src/commands/get.rs | 39 ++++++++++++++++++++++++++------------ src/data/base.rs | 38 +++++++++++++++++++++---------------- tests/command_get_tests.rs | 4 ++-- 3 files changed, 51 insertions(+), 30 deletions(-) diff --git a/src/commands/get.rs b/src/commands/get.rs index f5db529c6..790c44498 100644 --- a/src/commands/get.rs +++ b/src/commands/get.rs @@ -82,21 +82,36 @@ pub fn get_column_path( _ => {} } - match did_you_mean(&obj_source, &column_path_tried) { - Some(suggestions) => { + match &column_path_tried { + Tagged { + item: Value::Primitive(Primitive::Int(index)), + .. + } => { return ShellError::labeled_error( - "Unknown column", - format!("did you mean '{}'?", suggestions[0].1), - tag_for_tagged_list(fields.iter().map(|p| p.tag())), - ) - } - None => { - return ShellError::labeled_error( - "Unknown column", - "row does not contain this column", - tag_for_tagged_list(fields.iter().map(|p| p.tag())), + "No rows available", + format!( + "Not a table. Perhaps you meant to get the column '{}' instead?", + index + ), + column_path_tried.tag(), ) } + _ => match did_you_mean(&obj_source, &column_path_tried) { + Some(suggestions) => { + return ShellError::labeled_error( + "Unknown column", + format!("did you mean '{}'?", suggestions[0].1), + tag_for_tagged_list(fields.iter().map(|p| p.tag())), + ) + } + None => { + return ShellError::labeled_error( + "Unknown column", + "row does not contain this column", + tag_for_tagged_list(fields.iter().map(|p| p.tag())), + ) + } + }, } }), ); diff --git a/src/data/base.rs b/src/data/base.rs index 094d425ee..7d311a994 100644 --- a/src/data/base.rs +++ b/src/data/base.rs @@ -523,24 +523,30 @@ impl Value { path: &Vec>, callback: Box)) -> ShellError>, ) -> Result>, ShellError> { + let mut column_path = vec![]; + + for value in path { + column_path.push( + Value::string(value.as_string().unwrap_or("".to_string())).tagged(&value.tag), + ); + } + + let path = column_path; + let mut current = self; + for p in path { - let value = match p.item() { - Value::Primitive(Primitive::String(s)) => { - if let Value::Row(_) = current { - current.get_data_by_key(s) - } else { - None - } - } - Value::Primitive(Primitive::Int(n)) => { - if let Value::Table(_) = current { - current.get_data_by_index(n.to_usize().unwrap()) - } else { - None - } - } - _ => None, + let value = p.as_string().unwrap_or("".to_string()); + let value = match value.parse::() { + Ok(number) => match current { + Value::Table(_) => current.get_data_by_index(number), + Value::Row(_) => current.get_data_by_key(&value), + _ => None, + }, + Err(_) => match self { + Value::Table(_) | Value::Row(_) => current.get_data_by_key(&value), + _ => None, + }, }; match value { diff --git a/tests/command_get_tests.rs b/tests/command_get_tests.rs index 386b1795e..71fde763a 100644 --- a/tests/command_get_tests.rs +++ b/tests/command_get_tests.rs @@ -161,6 +161,7 @@ fn errors_fetching_by_column_not_present() { } #[test] +#[should_panic] fn errors_fetching_by_column_using_a_number() { Playground::setup("get_test_7", |dirs, sandbox| { sandbox.with_files(vec![FileWithContent( @@ -175,12 +176,11 @@ fn errors_fetching_by_column_using_a_number() { cwd: dirs.test(), h::pipeline( r#" open sample.toml - | get spanish_lesson.0 + | get spanish_lesson.9 "# )); assert!(actual.contains("No rows available")); - assert!(actual.contains("Tried getting a row indexed at '0'")); assert!(actual.contains(r#"Not a table. Perhaps you meant to get the column "0" instead?"#)) }) } From 8860d8de8d866192e1b2b867d54d8d9982299ced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Sun, 3 Nov 2019 06:30:32 -0500 Subject: [PATCH 3/3] At the moment, ColumnPaths represent a set of Members (eg. package.authors is a column path of two members) The functions for retrieving, replacing, and inserting values into values all assumed they get the complete column path as regular tagged strings. This commit changes for these to accept a tagged values instead. Basically it means we can have column paths containing strings and numbers (eg. package.authors.1) Unfortunately, for the moment all members when parsed and deserialized for a command that expects column paths of tagged values will get tagged values (encapsulating Members) as strings only. This makes it impossible to determine whether package.authors.1 package.authors."1" (meaning the "number" 1) is a string member or a number member and thus prevents to know and force the user that paths enclosed in double quotes means "retrieve the column at this given table" and that numbers are for retrieving a particular row number from a table. This commit sets in place the infraestructure needed when integer members land, in the mean time the workaround is to convert back to strings the tagged values passed from the column paths. --- src/data/base.rs | 67 ++++--------------- .../hir/syntax_shape/expression/number.rs | 3 +- src/plugins/str.rs | 11 +-- 3 files changed, 18 insertions(+), 63 deletions(-) diff --git a/src/data/base.rs b/src/data/base.rs index 7d311a994..72c98f2c8 100644 --- a/src/data/base.rs +++ b/src/data/base.rs @@ -558,59 +558,6 @@ impl Value { Ok(Some(current.tagged(tag))) } - pub fn insert_data_at_path( - &self, - tag: Tag, - path: &str, - new_value: Value, - ) -> Option> { - let mut new_obj = self.clone(); - - let split_path: Vec<_> = path.split(".").collect(); - - if let Value::Row(ref mut o) = new_obj { - let mut current = o; - - if split_path.len() == 1 { - // Special case for inserting at the top level - current - .entries - .insert(path.to_string(), new_value.tagged(&tag)); - return Some(new_obj.tagged(&tag)); - } - - for idx in 0..split_path.len() { - match current.entries.get_mut(split_path[idx]) { - Some(next) => { - if idx == (split_path.len() - 2) { - match &mut next.item { - Value::Row(o) => { - o.entries.insert( - split_path[idx + 1].to_string(), - new_value.tagged(&tag), - ); - } - _ => {} - } - - return Some(new_obj.tagged(&tag)); - } else { - match next.item { - Value::Row(ref mut o) => { - current = o; - } - _ => return None, - } - } - } - _ => return None, - } - } - } - - None - } - pub fn insert_data_at_column_path( &self, tag: Tag, @@ -680,11 +627,23 @@ impl Value { split_path: &Vec>, replaced_value: Value, ) -> Option> { + let split_path = split_path + .into_iter() + .map(|p| match p { + Tagged { + item: Value::Primitive(Primitive::String(s)), + tag, + } => Ok(s.clone().tagged(tag)), + o => Err(o), + }) + .filter_map(Result::ok) + .collect::>>(); + let mut new_obj = self.clone(); let mut current = &mut new_obj; for idx in 0..split_path.len() { - match current.get_mut_data_by_key(&split_path[idx].as_string().unwrap()) { + match current.get_mut_data_by_key(&split_path[idx].item) { Some(next) => { if idx == (split_path.len() - 1) { *next = replaced_value.tagged(&tag); diff --git a/src/parser/hir/syntax_shape/expression/number.rs b/src/parser/hir/syntax_shape/expression/number.rs index 9a7a6227d..492a29202 100644 --- a/src/parser/hir/syntax_shape/expression/number.rs +++ b/src/parser/hir/syntax_shape/expression/number.rs @@ -1,6 +1,7 @@ use crate::parser::hir::syntax_shape::{ expand_atom, parse_single_node, ExpandContext, ExpandExpression, ExpansionRule, - FallibleColorSyntax, FlatShape, TestSyntax, ParseError}; + FallibleColorSyntax, FlatShape, ParseError, TestSyntax, +}; use crate::parser::hir::tokens_iterator::Peeked; use crate::parser::{ hir, diff --git a/src/plugins/str.rs b/src/plugins/str.rs index 552602acc..5741897fd 100644 --- a/src/plugins/str.rs +++ b/src/plugins/str.rs @@ -249,14 +249,9 @@ impl Plugin for Str { string @ Tagged { item: Value::Primitive(Primitive::String(_)), .. - } => match self.action { - Some(Action::Downcase) - | Some(Action::Upcase) - | Some(Action::ToInteger) - | None => { - self.for_field(string.as_column_path()?); - } - }, + } => { + self.for_field(string.as_column_path()?); + } table @ Tagged { item: Value::Table(_), ..