From b54ce921dd67a1dcfa08045b1aa62fcca846600e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Thu, 31 Oct 2019 04:36:08 -0500 Subject: [PATCH] Better error messages. --- src/commands/get.rs | 20 ++++++ src/data/base.rs | 12 ++-- src/plugins/str.rs | 2 - tests/command_get_tests.rs | 125 +++++++++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 10 deletions(-) create mode 100644 tests/command_get_tests.rs diff --git a/src/commands/get.rs b/src/commands/get.rs index 69ef15333..cda637495 100644 --- a/src/commands/get.rs +++ b/src/commands/get.rs @@ -56,6 +56,26 @@ pub fn get_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(), + }; + + 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, + ); + } + _ => {} + } + match did_you_mean(&obj_source, &column_path_tried) { Some(suggestions) => { return ShellError::labeled_error( diff --git a/src/data/base.rs b/src/data/base.rs index 2d0288ac8..470a97f61 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() @@ -534,12 +533,9 @@ impl Value { ) -> Result>, ShellError> { let mut current = self; for p in path { - let value = if p.chars().all(char::is_numeric) { - current.get_data_by_index(p.chars().fold(0 as usize, |acc, c| { - c.to_digit(10).unwrap_or(0) as usize + acc - })) - } else { - 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 { diff --git a/src/plugins/str.rs b/src/plugins/str.rs index f565d5209..e6b047dad 100644 --- a/src/plugins/str.rs +++ b/src/plugins/str.rs @@ -98,8 +98,6 @@ impl Str { value.tag(), &f, Box::new(move |(obj_source, column_path_tried)| { - //let fields = f.clone(); - match did_you_mean(&obj_source, &column_path_tried) { Some(suggestions) => { return ShellError::labeled_error( diff --git a/tests/command_get_tests.rs b/tests/command_get_tests.rs new file mode 100644 index 000000000..e3c2272ac --- /dev/null +++ b/tests/command_get_tests.rs @@ -0,0 +1,125 @@ +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_2", |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("first_test_4", |dirs, sandbox| { + sandbox.with_files(vec![EmptyFile("andres.txt")]); + + let actual = nu_error!( + cwd: dirs.test(), "ls | get" + ); + + assert!(actual.contains("requires member parameter")); + }) +}