From c039e4b3d084143195622dfc043770dc57f61bb1 Mon Sep 17 00:00:00 2001 From: Eric Hodel Date: Tue, 7 Nov 2023 07:27:10 -0800 Subject: [PATCH] Update description and error types for `split-by` (#10865) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description `split-by` only works on a `Record`, the error type was updated to match, and now uses a more-specific type. (Two type fixes for the price of one!) The `usage` was updated to say "record" as well # User-Facing Changes * Providing the wrong type to `split-by` now gives an error messages with the correct required input type Previously: ``` ❯ ls | get name | split-by type Error: × unsupported input ╭─[entry #267:1:1] 1 │ ls | get name | split-by type · ─┬─ · ╰── requires a table with one row for splitting ╰──── ``` With this PR: ``` ❯ ls | get name | split-by type Error: nu::shell::type_mismatch × Type mismatch. ╭─[entry #1:1:1] 1 │ ls | get name | split-by type · ─┬─ · ╰── requires a record to split ╰──── ``` # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting Only generated commands need to be updated --------- Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com> --- crates/nu-command/src/filters/split_by.rs | 48 ++++++++++---------- crates/nu-command/tests/commands/split_by.rs | 21 +++++++-- 2 files changed, 41 insertions(+), 28 deletions(-) diff --git a/crates/nu-command/src/filters/split_by.rs b/crates/nu-command/src/filters/split_by.rs index 61f46452a8..f5f93b5aa8 100644 --- a/crates/nu-command/src/filters/split_by.rs +++ b/crates/nu-command/src/filters/split_by.rs @@ -23,7 +23,7 @@ impl Command for SplitBy { } fn usage(&self) -> &str { - "Create a new table split." + "Split a record into groups" } fn run( @@ -40,13 +40,13 @@ impl Command for SplitBy { vec![Example { description: "split items by column named \"lang\"", example: r#"{ - '2019': [ - { name: 'andres', lang: 'rb', year: '2019' }, - { name: 'jt', lang: 'rs', year: '2019' } - ], - '2021': [ - { name: 'storm', lang: 'rs', 'year': '2021' } - ] + '2019': [ + { name: 'andres', lang: 'rb', year: '2019' }, + { name: 'jt', lang: 'rs', year: '2019' } + ], + '2021': [ + { name: 'storm', lang: 'rs', 'year': '2021' } + ] } | split-by lang"#, result: Some(Value::test_record(record! { "rb" => Value::test_record(record! { @@ -176,7 +176,7 @@ fn data_group( pub fn data_split( value: PipelineData, splitter: Option<&dyn Fn(usize, &Value) -> Result>, - span: Span, + dst_span: Span, ) -> Result { let mut splits = indexmap::IndexMap::new(); @@ -202,33 +202,31 @@ pub fn data_split( } } _ => { - return Err(ShellError::GenericError( - "unsupported input".into(), - "requires a table with one row for splitting".into(), - Some(span), - None, - Vec::new(), - )) + return Err(ShellError::OnlySupportsThisInputType { + exp_input_type: "Record".into(), + wrong_type: v.get_type().to_string(), + dst_span, + src_span: v.span(), + }) } } } + PipelineData::Empty => return Err(ShellError::PipelineEmpty { dst_span }), _ => { - return Err(ShellError::GenericError( - "unsupported input".into(), - "requires a table with one row for splitting".into(), - Some(span), - None, - Vec::new(), - )) + return Err(ShellError::PipelineMismatch { + exp_input_type: "record".into(), + dst_span, + src_span: value.span().unwrap_or(Span::unknown()), + }) } } let record = splits .into_iter() - .map(|(k, rows)| (k, Value::record(rows.into_iter().collect(), span))) + .map(|(k, rows)| (k, Value::record(rows.into_iter().collect(), dst_span))) .collect(); - Ok(PipelineData::Value(Value::record(record, span), None)) + Ok(PipelineData::Value(Value::record(record, dst_span), None)) } #[cfg(test)] diff --git a/crates/nu-command/tests/commands/split_by.rs b/crates/nu-command/tests/commands/split_by.rs index cafcf22da7..671f136232 100644 --- a/crates/nu-command/tests/commands/split_by.rs +++ b/crates/nu-command/tests/commands/split_by.rs @@ -31,7 +31,16 @@ fn splits() { } #[test] -fn errors_if_no_table_given_as_input() { +fn errors_if_no_input() { + Playground::setup("split_by_no_input", |dirs, _sandbox| { + let actual = nu!(cwd: dirs.test(), pipeline("split-by type")); + + assert!(actual.err.contains("no input value was piped in")); + }) +} + +#[test] +fn errors_if_non_record_input() { Playground::setup("split_by_test_2", |dirs, sandbox| { sandbox.with_files(vec![ EmptyFile("los.txt"), @@ -40,7 +49,11 @@ fn errors_if_no_table_given_as_input() { EmptyFile("arepas.clu"), ]); - let actual = nu!( + let input_mismatch = nu!(cwd: dirs.test(), pipeline("5 | split-by type")); + + assert!(input_mismatch.err.contains("doesn't support int input")); + + let only_supports = nu!( cwd: dirs.test(), pipeline( " ls @@ -49,6 +62,8 @@ fn errors_if_no_table_given_as_input() { " )); - assert!(actual.err.contains("requires a table")); + assert!(only_supports + .err + .contains("only Record input data is supported")); }) }