diff --git a/src/commands/get.rs b/src/commands/get.rs index 70508bdb7a..cda637495e 100644 --- a/src/commands/get.rs +++ b/src/commands/get.rs @@ -1,8 +1,8 @@ use crate::commands::WholeStreamCommand; -use crate::data::meta::tag_for_tagged_list; use crate::data::Value; use crate::errors::ShellError; use crate::prelude::*; +use crate::utils::did_you_mean; use log::trace; pub struct Get; @@ -50,56 +50,71 @@ pub fn get_column_path( path: &ColumnPath, obj: &Tagged, ) -> Result, ShellError> { - let mut current = Some(obj); - for p in path.iter() { - if let Some(obj) = current { - current = match obj.get_data_by_key(&p) { - Some(v) => Some(v), - None => - // Before we give up, see if they gave us a path that matches a field name by itself - { - let possibilities = obj.data_descriptors(); + let fields = path.clone(); - let mut possible_matches: Vec<_> = possibilities - .iter() - .map(|x| (natural::distance::levenshtein_distance(x, &p), x)) - .collect(); + let value = obj.get_data_by_column_path( + obj.tag(), + path, + Box::new(move |(obj_source, column_path_tried)| { + match obj_source { + Value::Table(rows) => { + let total = rows.len(); + let end_tag = match fields.iter().nth_back(if fields.len() > 2 { 1 } else { 0 }) + { + Some(last_field) => last_field.tag(), + None => column_path_tried.tag(), + }; - possible_matches.sort(); + return ShellError::labeled_error_with_secondary( + "Row not found", + format!("There isn't a row indexed at '{}'", **column_path_tried), + column_path_tried.tag(), + format!("The table only has {} rows (0..{})", total, total - 1), + end_tag, + ); + } + _ => {} + } - if possible_matches.len() > 0 { - return Err(ShellError::labeled_error( - "Unknown column", - format!("did you mean '{}'?", possible_matches[0].1), - tag_for_tagged_list(path.iter().map(|p| p.tag())), - )); - } else { - return Err(ShellError::labeled_error( - "Unknown column", - "row does not contain this column", - tag_for_tagged_list(path.iter().map(|p| p.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())), + ) } } - } - } + }), + ); - match current { - Some(v) => Ok(v.clone()), - None => match obj { - // If its None check for certain values. - Tagged { - item: Value::Primitive(Primitive::String(_)), - .. - } => Ok(obj.clone()), - Tagged { - item: Value::Primitive(Primitive::Path(_)), - .. - } => Ok(obj.clone()), - _ => Ok(Value::nothing().tagged(&obj.tag)), + let res = match value { + Ok(fetched) => match fetched { + Some(Tagged { item: v, tag }) => Ok((v.clone()).tagged(&tag)), + None => match obj { + // If its None check for certain values. + Tagged { + item: Value::Primitive(Primitive::String(_)), + .. + } => Ok(obj.clone()), + Tagged { + item: Value::Primitive(Primitive::Path(_)), + .. + } => Ok(obj.clone()), + _ => Ok(Value::nothing().tagged(&obj.tag)), + }, }, - } + Err(reason) => Err(reason), + }; + + res } pub fn get( @@ -118,26 +133,30 @@ pub fn get( let member = vec![member.clone()]; - let fields = vec![&member, &fields] + let column_paths = vec![&member, &fields] .into_iter() .flatten() .collect::>(); - for column_path in &fields { - match get_column_path(column_path, &item) { - Ok(Tagged { - item: Value::Table(l), - .. - }) => { - for item in l { - result.push_back(ReturnSuccess::value(item.clone())); + for path in column_paths { + let res = get_column_path(&path, &item); + + match res { + Ok(got) => match got { + Tagged { + item: Value::Table(rows), + .. + } => { + for item in rows { + result.push_back(ReturnSuccess::value(item.clone())); + } } - } - Ok(x) => result.push_back(ReturnSuccess::value(x.clone())), - Err(x) => result.push_back(Err(x)), + other => result + .push_back(ReturnSuccess::value((*other).clone().tagged(&item.tag))), + }, + Err(reason) => result.push_back(Err(reason)), } } - result }) .flatten(); diff --git a/src/data/base.rs b/src/data/base.rs index bc567f0dfe..470a97f617 100644 --- a/src/data/base.rs +++ b/src/data/base.rs @@ -459,11 +459,10 @@ impl Value { } } - // TODO: This is basically a legacy construct, I think pub fn data_descriptors(&self) -> Vec { match self { Value::Primitive(_) => vec![], - Value::Row(o) => o + Value::Row(columns) => columns .entries .keys() .into_iter() @@ -475,6 +474,13 @@ impl Value { } } + pub(crate) fn get_data_by_index(&self, idx: usize) -> Option<&Tagged> { + match self { + Value::Table(value_set) => value_set.get(idx), + _ => None, + } + } + pub(crate) fn get_data_by_key(&self, name: &str) -> Option<&Tagged> { match self { Value::Row(o) => o.get_data_by_key(name), @@ -523,16 +529,22 @@ impl Value { &self, tag: Tag, path: &Vec>, - ) -> Option> { + callback: Box)) -> ShellError>, + ) -> Result>, ShellError> { let mut current = self; for p in path { - match current.get_data_by_key(p) { + let value = match p.item().parse::() { + Ok(number) => current.get_data_by_index(number), + Err(_) => current.get_data_by_key(p), + }; + + match value { Some(v) => current = v, - None => return None, + None => return Err(callback((¤t.clone(), &p.clone()))), } } - Some(current.tagged(tag)) + Ok(Some(current.tagged(tag))) } pub fn insert_data_at_path( @@ -912,6 +924,7 @@ fn coerce_compare_primitive( mod tests { use crate::data::meta::*; + use crate::ShellError; use crate::Value; use indexmap::IndexMap; @@ -927,6 +940,10 @@ mod tests { Value::table(list).tagged_unknown() } + 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 @@ -960,7 +977,7 @@ mod tests { let (version, tag) = string("0.4.0").into_parts(); - let row = Value::row(indexmap! { + let value = Value::row(indexmap! { "package".into() => row(indexmap! { "name".into() => string("nu"), @@ -969,7 +986,10 @@ mod tests { }); assert_eq!( - **row.get_data_by_column_path(tag, &field_path).unwrap(), + **value + .get_data_by_column_path(tag, &field_path, Box::new(error_callback())) + .unwrap() + .unwrap(), version ) } @@ -980,7 +1000,7 @@ mod tests { let (name, tag) = string("Andrés N. Robalino").into_parts(); - let row = Value::row(indexmap! { + let value = Value::row(indexmap! { "package".into() => row(indexmap! { "name".into() => string("nu"), "version".into() => string("0.4.0"), @@ -993,11 +1013,43 @@ mod tests { }); assert_eq!( - **row.get_data_by_column_path(tag, &field_path).unwrap(), + **value + .get_data_by_column_path(tag, &field_path, Box::new(error_callback())) + .unwrap() + .unwrap(), name ) } + #[test] + fn column_path_that_contains_just_a_numbers_gets_a_row_from_a_table() { + let field_path = column_path(&vec![string("package"), string("authors"), string("0")]); + + let (_, 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(), + Value::row(indexmap! { + "name".into() => string("Andrés N. Robalino") + }) + ); + } + #[test] fn replaces_matching_field_from_a_row() { let field_path = column_path(&vec![string("amigos")]); diff --git a/src/lib.rs b/src/lib.rs index 520e08a136..f21a70cfe2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,12 +30,12 @@ pub use crate::env::host::BasicHost; pub use crate::parser::hir::SyntaxShape; pub use crate::parser::parse::token_tree_builder::TokenTreeBuilder; pub use crate::plugin::{serve_plugin, Plugin}; -pub use crate::utils::{AbsoluteFile, AbsolutePath, RelativePath}; +pub use crate::utils::{did_you_mean, AbsoluteFile, AbsolutePath, RelativePath}; pub use cli::cli; pub use data::base::{Primitive, Value}; pub use data::config::{config_path, APP_INFO}; pub use data::dict::{Dictionary, TaggedDictBuilder}; -pub use data::meta::{Span, Spanned, SpannedItem, Tag, Tagged, TaggedItem}; +pub use data::meta::{tag_for_tagged_list, Span, Spanned, SpannedItem, Tag, Tagged, TaggedItem}; pub use errors::{CoerceInto, ShellError}; pub use num_traits::cast::ToPrimitive; pub use parser::parse::text::Text; diff --git a/src/plugins/inc.rs b/src/plugins/inc.rs index ed0416ce43..fb3836dfd3 100644 --- a/src/plugins/inc.rs +++ b/src/plugins/inc.rs @@ -1,6 +1,6 @@ use nu::{ - serve_plugin, CallInfo, Plugin, Primitive, ReturnSuccess, ReturnValue, ShellError, Signature, - SyntaxShape, Tagged, TaggedItem, Value, + did_you_mean, serve_plugin, tag_for_tagged_list, CallInfo, Plugin, Primitive, ReturnSuccess, + ReturnValue, ShellError, Signature, SyntaxShape, Tagged, TaggedItem, Value, }; enum Action { @@ -93,22 +93,51 @@ impl Inc { )); } } + Value::Row(_) => match self.field { Some(ref f) => { - let replacement = match value.item.get_data_by_column_path(value.tag(), f) { - Some(result) => self.inc(result.map(|x| x.clone()))?, - None => { - return Err(ShellError::labeled_error( - "inc could not find field to replace", - "column name", - value.tag(), - )) - } + let fields = f.clone(); + + let replace_for = value.item.get_data_by_column_path( + value.tag(), + &f, + Box::new(move |(obj_source, column_path_tried)| { + 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())), + ) + } + } + }), + ); + + let replacement = match replace_for { + Ok(got) => match got { + Some(result) => self.inc(result.map(|x| x.clone()))?, + None => { + return Err(ShellError::labeled_error( + "inc could not find field to replace", + "column name", + value.tag(), + )) + } + }, + Err(reason) => return Err(reason), }; match value.item.replace_data_at_column_path( value.tag(), - f, + &f, replacement.item.clone(), ) { Some(v) => return Ok(v), diff --git a/src/plugins/str.rs b/src/plugins/str.rs index 8260bdac2c..e6b047dad3 100644 --- a/src/plugins/str.rs +++ b/src/plugins/str.rs @@ -1,6 +1,6 @@ use nu::{ - serve_plugin, CallInfo, Plugin, Primitive, ReturnSuccess, ReturnValue, ShellError, Signature, - SyntaxShape, Tagged, TaggedItem, Value, + did_you_mean, serve_plugin, tag_for_tagged_list, CallInfo, Plugin, Primitive, ReturnSuccess, + ReturnValue, ShellError, Signature, SyntaxShape, Tagged, TaggedItem, Value, }; #[derive(Debug, Eq, PartialEq)] @@ -92,13 +92,48 @@ impl Str { Value::Primitive(Primitive::String(ref s)) => Ok(self.apply(&s)?.tagged(value.tag())), Value::Row(_) => match self.field { Some(ref f) => { - let replacement = match value.item.get_data_by_column_path(value.tag(), f) { - Some(result) => self.strutils(result.map(|x| x.clone()))?, - None => return Ok(Value::nothing().tagged(value.tag)), + let fields = f.clone(); + + let replace_for = value.item.get_data_by_column_path( + value.tag(), + &f, + Box::new(move |(obj_source, column_path_tried)| { + 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())), + ) + } + } + }), + ); + + let replacement = match replace_for { + Ok(got) => match got { + Some(result) => self.strutils(result.map(|x| x.clone()))?, + None => { + return Err(ShellError::labeled_error( + "inc could not find field to replace", + "column name", + value.tag(), + )) + } + }, + Err(reason) => return Err(reason), }; + match value.item.replace_data_at_column_path( value.tag(), - f, + &f, replacement.item.clone(), ) { Some(v) => return Ok(v), diff --git a/src/prelude.rs b/src/prelude.rs index 4b12a07bda..6ff62c3240 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -66,7 +66,9 @@ pub(crate) use crate::commands::RawCommandArgs; pub(crate) use crate::context::CommandRegistry; pub(crate) use crate::context::{AnchorLocation, Context}; pub(crate) use crate::data::base as value; -pub(crate) use crate::data::meta::{Span, Spanned, SpannedItem, Tag, Tagged, TaggedItem}; +pub(crate) use crate::data::meta::{ + tag_for_tagged_list, Span, Spanned, SpannedItem, Tag, Tagged, TaggedItem, +}; pub(crate) use crate::data::types::ExtractType; pub(crate) use crate::data::{Primitive, Value}; pub(crate) use crate::env::host::handle_unexpected; diff --git a/src/utils.rs b/src/utils.rs index 56fee491b6..9822b76278 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -5,6 +5,30 @@ use std::fmt; use std::ops::Div; use std::path::{Component, Path, PathBuf}; +pub fn did_you_mean( + obj_source: &Value, + field_tried: &Tagged, +) -> Option> { + let possibilities = obj_source.data_descriptors(); + + let mut possible_matches: Vec<_> = possibilities + .into_iter() + .map(|x| { + let word = x.clone(); + let distance = natural::distance::levenshtein_distance(&word, &field_tried); + + (distance, word) + }) + .collect(); + + if possible_matches.len() > 0 { + possible_matches.sort(); + return Some(possible_matches); + } + + None +} + pub struct AbsoluteFile { inner: PathBuf, } diff --git a/tests/command_get_tests.rs b/tests/command_get_tests.rs new file mode 100644 index 0000000000..a6a4dea8e3 --- /dev/null +++ b/tests/command_get_tests.rs @@ -0,0 +1,126 @@ +mod helpers; + +use helpers as h; +use helpers::{Playground, Stub::*}; + +#[test] +fn get() { + Playground::setup("get_test_1", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "sample.toml", + r#" + nu_party_venue = "zion" + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), h::pipeline( + r#" + open sample.toml + | get nu_party_venue + | echo $it + "# + )); + + assert_eq!(actual, "zion"); + }) +} + +#[test] +fn fetches_by_index_from_a_given_table() { + Playground::setup("get_test_2", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "sample.toml", + r#" + [package] + name = "nu" + version = "0.4.1" + authors = ["Yehuda Katz ", "Jonathan Turner ", "Andrés N. Robalino "] + description = "When arepas shells are tasty and fun." + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), h::pipeline( + r#" + open sample.toml + | get package.authors.2 + | echo $it + "# + )); + + assert_eq!(actual, "Andrés N. Robalino "); + }) +} + +#[test] +fn fetches_more_than_one_column_member_path() { + Playground::setup("get_test_3", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "sample.toml", + r#" + [[fortune_tellers]] + name = "Andrés N. Robalino" + arepas = 1 + + [[fortune_tellers]] + name = "Jonathan Turner" + arepas = 1 + + [[fortune_tellers]] + name = "Yehuda Katz" + arepas = 1 + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), h::pipeline( + r#" + open sample.toml + | get fortune_tellers.2.name fortune_tellers.0.name fortune_tellers.1.name + | nth 2 + | echo $it + "# + )); + + assert_eq!(actual, "Jonathan Turner"); + }) +} + +#[test] +fn errors_fetching_by_index_out_of_bounds_from_table() { + Playground::setup("get_test_4", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "sample.toml", + r#" + [spanish_lesson] + sentence_words = ["Yo", "quiero", "taconushell"] + "#, + )]); + + let actual = nu_error!( + cwd: dirs.test(), h::pipeline( + r#" + open sample.toml + | get spanish_lesson.sentence_words.3 + "# + )); + + assert!(actual.contains("Row not found")); + assert!(actual.contains("There isn't a row indexed at '3'")); + assert!(actual.contains("The table only has 3 rows (0..2)")) + }) +} + +#[test] +fn requires_at_least_one_column_member_path() { + Playground::setup("get_test_5", |dirs, sandbox| { + sandbox.with_files(vec![EmptyFile("andres.txt")]); + + let actual = nu_error!( + cwd: dirs.test(), "ls | get" + ); + + assert!(actual.contains("requires member parameter")); + }) +}