extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2025-03-31 11:17:25 +02:00
Code Issues Packages Projects Releases Wiki Activity

Improve CantFindColumn and ColumnAlreadyExists errors (#7164)

Browse Source 
* Improve CantFindColumn and ColumnAlreadyExists errors

* Update tests
This commit is contained in:
Leon 2022-11-20 03:35:55 +10:00 committed by GitHub
parent 41f72b1236
commit 4b83a2d27a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 131 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
crates/nu-command/src/filters/group_by.rs
View File

@ -250,6 +250,7 @@ pub fn group(
move |_, row: &Value| match row.get_data_by_key(&column_name.item) { move |_, row: &Value| match row.get_data_by_key(&column_name.item) {
Some(group_key) => Ok(group_key.as_string()?), Some(group_key) => Ok(group_key.as_string()?),
None => Err(ShellError::CantFindColumn( None => Err(ShellError::CantFindColumn(
column_name.item.to_string(),
column_name.span, column_name.span,
row.span().unwrap_or(column_name.span), row.span().unwrap_or(column_name.span),
)), )),

2
crates/nu-command/src/filters/split_by.rs
View File

@ -137,6 +137,7 @@ pub fn split_by(
}); });
Ok(split(&splitter, input, name)?) Ok(split(&splitter, input, name)?)
} }
// This uses the same format as the 'requires a column name' error in sort_utils.rs
None => Err(ShellError::GenericError( None => Err(ShellError::GenericError(
"expected name".into(), "expected name".into(),
"requires a column name for splitting".into(), "requires a column name for splitting".into(),
@ -165,6 +166,7 @@ pub fn split(
move |_, row: &Value| match row.get_data_by_key(&column_name.item) { move |_, row: &Value| match row.get_data_by_key(&column_name.item) {
Some(group_key) => Ok(group_key.as_string()?), Some(group_key) => Ok(group_key.as_string()?),
None => Err(ShellError::CantFindColumn( None => Err(ShellError::CantFindColumn(
column_name.item.to_string(),
column_name.span, column_name.span,
row.span().unwrap_or(column_name.span), row.span().unwrap_or(column_name.span),
)), )),

16
crates/nu-command/src/sort_utils.rs
View File

@ -1,5 +1,5 @@
use alphanumeric_sort::compare_str; use alphanumeric_sort::compare_str;
use nu_engine::column::column_does_not_exist; use nu_engine::column::nonexistent_column;
use nu_protocol::{ShellError, Span, Value}; use nu_protocol::{ShellError, Span, Value};
use std::cmp::Ordering; use std::cmp::Ordering;
@ -81,12 +81,18 @@ pub fn sort(
.. ..
} => { } => {
if sort_columns.is_empty() { if sort_columns.is_empty() {
println!("sort-by requires a column name to sort table data"); // This uses the same format as the 'requires a column name' error in split_by.rs
return Err(ShellError::CantFindColumn(span, span)); return Err(ShellError::GenericError(
"expected name".into(),
"requires a column name to sort table data".into(),
Some(span),
None,
Vec::new(),
));
} }
if column_does_not_exist(sort_columns.clone(), cols.to_vec()) { if let Some(nonexistent) = nonexistent_column(sort_columns.clone(), cols.to_vec()) {
return Err(ShellError::CantFindColumn(span, span)); return Err(ShellError::CantFindColumn(nonexistent, span, span));
} }
// check to make sure each value in each column in the record // check to make sure each value in each column in the record

4
crates/nu-command/tests/commands/insert.rs
View File

@ -24,7 +24,9 @@ fn insert_the_column_conflict() {
"# "#
)); ));
assert!(actual.err.contains("column already exists")); assert!(actual
.err
.contains("column 'pretty_assertions' already exists"));
} }
#[test] #[test]

10
crates/nu-command/tests/commands/update.rs
View File

@ -95,3 +95,13 @@ fn update_past_end_list() {
assert!(actual.err.contains("too large")); assert!(actual.err.contains("too large"));
} }
#[test]
fn update_nonexistent_column() {
let actual = nu!(
cwd: ".", pipeline(
r#"{a:1} | update b 2"#
));
assert!(actual.err.contains("cannot find column 'b'"));
}

17
crates/nu-engine/src/column.rs
View File

@ -19,22 +19,15 @@ pub fn get_columns<'a>(input: impl IntoIterator<Item = &'a Value>) -> Vec<String
columns columns
} }
/* // If a column doesn't exist in the input, return it.
* Check to see if any of the columns inside the input pub fn nonexistent_column(inputs: Vec<String>, columns: Vec<String>) -> Option<String> {
* does not exist in a vec of columns let set: HashSet<String> = HashSet::from_iter(columns.iter().cloned());
*/
pub fn column_does_not_exist(inputs: Vec<String>, columns: Vec<String>) -> bool {
let mut set = HashSet::new();
for column in columns {
set.insert(column);
}
for input in &inputs { for input in &inputs {
if set.contains(input) { if set.contains(input) {
continue; continue;
} }
return true; return Some(input.clone());
} }
false None
} }

6
crates/nu-protocol/src/shell_error.rs
View File

@ -411,7 +411,8 @@ Either make sure {0} is a string, or add a 'to_string' entry for it in ENV_CONVE
#[error("Cannot find column")] #[error("Cannot find column")]
#[diagnostic(code(nu::shell::column_not_found), url(docsrs))] #[diagnostic(code(nu::shell::column_not_found), url(docsrs))]
CantFindColumn( CantFindColumn(
#[label = "cannot find column"] Span, String,
#[label = "cannot find column '{0}'"] Span,
#[label = "value originates here"] Span, #[label = "value originates here"] Span,
), ),
@ -423,7 +424,8 @@ Either make sure {0} is a string, or add a 'to_string' entry for it in ENV_CONVE
#[error("Column already exists")] #[error("Column already exists")]
#[diagnostic(code(nu::shell::column_already_exists), url(docsrs))] #[diagnostic(code(nu::shell::column_already_exists), url(docsrs))]
ColumnAlreadyExists( ColumnAlreadyExists(
#[label = "column already exists"] Span, String,
#[label = "column '{0}' already exists"] Span,
#[label = "value originates here"] Span, #[label = "value originates here"] Span,
), ),

112
crates/nu-protocol/src/value/mod.rs
View File

@ -718,7 +718,11 @@ impl Value {
return Err(ShellError::DidYouMean(suggestion, *origin_span)); return Err(ShellError::DidYouMean(suggestion, *origin_span));
} }
} }
return Err(ShellError::CantFindColumn(*origin_span, span)); return Err(ShellError::CantFindColumn(
column_name.to_string(),
*origin_span,
span,
));
} }
} }
Value::List { vals, span } => { Value::List { vals, span } => {
@ -826,7 +830,13 @@ impl Value {
} }
} }
} }
v => return Err(ShellError::CantFindColumn(*span, v.span()?)), v => {
return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
v.span()?,
))
}
} }
} }
} }
@ -856,7 +866,13 @@ impl Value {
} }
} }
} }
v => return Err(ShellError::CantFindColumn(*span, v.span()?)), v => {
return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
v.span()?,
))
}
}, },
PathMember::Int { val: row_num, span } => match self { PathMember::Int { val: row_num, span } => match self {
Value::List { vals, .. } => { Value::List { vals, .. } => {
@ -924,10 +940,20 @@ impl Value {
} }
} }
if !found { if !found {
return Err(ShellError::CantFindColumn(*span, *v_span)); return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
*v_span,
));
} }
} }
v => return Err(ShellError::CantFindColumn(*span, v.span()?)), v => {
return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
v.span()?,
))
}
} }
} }
} }
@ -947,10 +973,20 @@ impl Value {
} }
} }
if !found { if !found {
return Err(ShellError::CantFindColumn(*span, *v_span)); return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
*v_span,
));
} }
} }
v => return Err(ShellError::CantFindColumn(*span, v.span()?)), v => {
return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
v.span()?,
))
}
}, },
PathMember::Int { val: row_num, span } => match self { PathMember::Int { val: row_num, span } => match self {
Value::List { vals, .. } => { Value::List { vals, .. } => {
@ -999,10 +1035,20 @@ impl Value {
} }
} }
if !found { if !found {
return Err(ShellError::CantFindColumn(*span, *v_span)); return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
*v_span,
));
} }
} }
v => return Err(ShellError::CantFindColumn(*span, v.span()?)), v => {
return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
v.span()?,
))
}
} }
} }
Ok(()) Ok(())
@ -1021,11 +1067,19 @@ impl Value {
} }
} }
if !found { if !found {
return Err(ShellError::CantFindColumn(*span, *v_span)); return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
*v_span,
));
} }
Ok(()) Ok(())
} }
v => Err(ShellError::CantFindColumn(*span, v.span()?)), v => Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
v.span()?,
)),
}, },
PathMember::Int { val: row_num, span } => match self { PathMember::Int { val: row_num, span } => match self {
Value::List { vals, .. } => { Value::List { vals, .. } => {
@ -1065,10 +1119,20 @@ impl Value {
} }
} }
if !found { if !found {
return Err(ShellError::CantFindColumn(*span, *v_span)); return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
*v_span,
));
} }
} }
v => return Err(ShellError::CantFindColumn(*span, v.span()?)), v => {
return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
v.span()?,
))
}
} }
} }
Ok(()) Ok(())
@ -1088,11 +1152,19 @@ impl Value {
} }
} }
if !found { if !found {
return Err(ShellError::CantFindColumn(*span, *v_span)); return Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
*v_span,
));
} }
Ok(()) Ok(())
} }
v => Err(ShellError::CantFindColumn(*span, v.span()?)), v => Err(ShellError::CantFindColumn(
col_name.to_string(),
*span,
v.span()?,
)),
}, },
PathMember::Int { val: row_num, span } => match self { PathMember::Int { val: row_num, span } => match self {
Value::List { vals, .. } => { Value::List { vals, .. } => {
@ -1134,7 +1206,9 @@ impl Value {
if col.0 == col_name { if col.0 == col_name {
if cell_path.len() == 1 { if cell_path.len() == 1 {
return Err(ShellError::ColumnAlreadyExists( return Err(ShellError::ColumnAlreadyExists(
*span, *v_span, col_name.to_string(),
*span,
*v_span,
)); ));
} else { } else {
return col.1.insert_data_at_cell_path( return col.1.insert_data_at_cell_path(
@ -1165,7 +1239,11 @@ impl Value {
for col in cols.iter().zip(vals.iter_mut()) { for col in cols.iter().zip(vals.iter_mut()) {
if col.0 == col_name { if col.0 == col_name {
if cell_path.len() == 1 { if cell_path.len() == 1 {
return Err(ShellError::ColumnAlreadyExists(*span, *v_span)); return Err(ShellError::ColumnAlreadyExists(
col_name.to_string(),
*span,
*v_span,
));
} else { } else {
return col return col
.1 .1