From 5a1d81221fbf1d2c2edfebe71111790e3a7a0a55 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Wed, 9 Feb 2022 09:59:40 -0500 Subject: [PATCH] Move 'nth' into 'select' (#4385) --- crates/nu-command/src/core_commands/tutor.rs | 8 +- crates/nu-command/src/default_context.rs | 1 - crates/nu-command/src/filters/columns.rs | 2 +- crates/nu-command/src/filters/mod.rs | 2 - crates/nu-command/src/filters/nth.rs | 152 ---------------- crates/nu-command/src/filters/select.rs | 164 ++++++++++++++---- crates/nu-command/tests/commands/get.rs | 4 +- crates/nu-command/tests/commands/mod.rs | 1 - .../nu-command/tests/commands/move_/column.rs | 2 +- crates/nu-command/tests/commands/nth.rs | 41 ----- crates/nu-command/tests/commands/open.rs | 8 +- crates/nu-command/tests/commands/parse.rs | 2 +- crates/nu-command/tests/commands/select.rs | 40 ++++- .../tests/format_conversions/sqlite.rs | 2 +- .../tests/format_conversions/tsv.rs | 2 +- crates/nu-protocol/src/value/mod.rs | 6 +- docs/commands/lines.md | 2 +- docs/commands/nth.md | 10 +- 18 files changed, 190 insertions(+), 259 deletions(-) delete mode 100644 crates/nu-command/src/filters/nth.rs delete mode 100644 crates/nu-command/tests/commands/nth.rs diff --git a/crates/nu-command/src/core_commands/tutor.rs b/crates/nu-command/src/core_commands/tutor.rs index 818ccb89db..574dd53fdb 100644 --- a/crates/nu-command/src/core_commands/tutor.rs +++ b/crates/nu-command/src/core_commands/tutor.rs @@ -176,10 +176,10 @@ using Nushell commands. To get the 3rd row in the table, you can use the `nth` command: ``` -ls | nth 2 +ls | select 2 ``` -This will get the 3rd (note that `nth` is zero-based) row in the table created -by the `ls` command. You can use `nth` on any table created by other commands +This will get the 3rd (note that `select` is zero-based) row in the table created +by the `ls` command. You can use `select` on any table created by other commands as well. You can also access the column of data in one of two ways. If you want @@ -218,7 +218,7 @@ like lists and tables. To reach a cell of data from a table, you can combine a row operation and a column operation. ``` -ls | nth 4 | get name +ls | select 4 | get name ``` You can combine these operations into one step using a shortcut. ``` diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index 1e53a6bac3..e2bde0420a 100644 --- a/crates/nu-command/src/default_context.rs +++ b/crates/nu-command/src/default_context.rs @@ -84,7 +84,6 @@ pub fn create_default_context(cwd: impl AsRef) -> EngineState { Last, Length, Lines, - Nth, ParEach, ParEachGroup, Prepend, diff --git a/crates/nu-command/src/filters/columns.rs b/crates/nu-command/src/filters/columns.rs index 303251a0f5..f6ece34815 100644 --- a/crates/nu-command/src/filters/columns.rs +++ b/crates/nu-command/src/filters/columns.rs @@ -35,7 +35,7 @@ impl Command for Columns { result: None, }, Example { - example: "[[name,age,grade]; [bill,20,a]] | columns | nth 1", + example: "[[name,age,grade]; [bill,20,a]] | columns | select 1", description: "Get the second column from the table", result: None, }, diff --git a/crates/nu-command/src/filters/mod.rs b/crates/nu-command/src/filters/mod.rs index 050f979b7c..049d839026 100644 --- a/crates/nu-command/src/filters/mod.rs +++ b/crates/nu-command/src/filters/mod.rs @@ -22,7 +22,6 @@ mod length; mod lines; mod merge; mod move_; -mod nth; mod par_each; mod par_each_group; mod prepend; @@ -69,7 +68,6 @@ pub use length::Length; pub use lines::Lines; pub use merge::Merge; pub use move_::Move; -pub use nth::Nth; pub use par_each::ParEach; pub use par_each_group::ParEachGroup; pub use prepend::Prepend; diff --git a/crates/nu-command/src/filters/nth.rs b/crates/nu-command/src/filters/nth.rs deleted file mode 100644 index d49ffda0d5..0000000000 --- a/crates/nu-command/src/filters/nth.rs +++ /dev/null @@ -1,152 +0,0 @@ -use nu_engine::CallExt; -use nu_protocol::ast::Call; -use nu_protocol::engine::{Command, EngineState, Stack}; -use nu_protocol::{ - Category, Example, IntoInterruptiblePipelineData, PipelineData, PipelineIterator, ShellError, - Signature, Span, SyntaxShape, Value, -}; - -#[derive(Clone)] -pub struct Nth; - -impl Command for Nth { - fn name(&self) -> &str { - "nth" - } - - fn signature(&self) -> Signature { - Signature::build("nth") - .rest("rest", SyntaxShape::Int, "the number of the row to return") - .switch("skip", "Skip the rows instead of selecting them", Some('s')) - .category(Category::Filters) - } - - fn usage(&self) -> &str { - "Return or skip only the selected rows." - } - - fn examples(&self) -> Vec { - vec![ - Example { - example: "[sam,sarah,2,3,4,5] | nth 0 1 2", - description: "Get the first, second, and third row", - result: Some(Value::List { - vals: vec![ - Value::test_string("sam"), - Value::test_string("sarah"), - Value::test_int(2), - ], - span: Span::test_data(), - }), - }, - Example { - example: "[0,1,2,3,4,5] | nth 0 1 2", - description: "Get the first, second, and third row", - result: Some(Value::List { - vals: vec![Value::test_int(0), Value::test_int(1), Value::test_int(2)], - span: Span::test_data(), - }), - }, - Example { - example: "[0,1,2,3,4,5] | nth -s 0 1 2", - description: "Skip the first, second, and third row", - result: Some(Value::List { - vals: vec![Value::test_int(3), Value::test_int(4), Value::test_int(5)], - span: Span::test_data(), - }), - }, - Example { - example: "[0,1,2,3,4,5] | nth 0 2 4", - description: "Get the first, third, and fifth row", - result: Some(Value::List { - vals: vec![Value::test_int(0), Value::test_int(2), Value::test_int(4)], - span: Span::test_data(), - }), - }, - Example { - example: "[0,1,2,3,4,5] | nth 2 0 4", - description: "Get the first, third, and fifth row", - result: Some(Value::List { - vals: vec![Value::test_int(0), Value::test_int(2), Value::test_int(4)], - span: Span::test_data(), - }), - }, - ] - } - - fn run( - &self, - engine_state: &EngineState, - stack: &mut Stack, - call: &Call, - input: PipelineData, - ) -> Result { - let mut rows: Vec = call.rest(engine_state, stack, 0)?; - rows.sort_unstable(); - let skip = call.has_flag("skip"); - let pipeline_iter: PipelineIterator = input.into_iter(); - - Ok(NthIterator { - input: pipeline_iter, - rows, - skip, - current: 0, - } - .into_pipeline_data(engine_state.ctrlc.clone())) - } -} - -struct NthIterator { - input: PipelineIterator, - rows: Vec, - skip: bool, - current: usize, -} - -impl Iterator for NthIterator { - type Item = Value; - - fn next(&mut self) -> Option { - loop { - if !self.skip { - if let Some(row) = self.rows.get(0) { - if self.current == *row { - self.rows.remove(0); - self.current += 1; - return self.input.next(); - } else { - self.current += 1; - let _ = self.input.next(); - continue; - } - } else { - return None; - } - } else if let Some(row) = self.rows.get(0) { - if self.current == *row { - self.rows.remove(0); - self.current += 1; - let _ = self.input.next(); - continue; - } else { - self.current += 1; - return self.input.next(); - } - } else { - return self.input.next(); - } - } - } -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn test_examples() { - use crate::test_examples; - - test_examples(Nth {}) - } -} diff --git a/crates/nu-command/src/filters/select.rs b/crates/nu-command/src/filters/select.rs index da71e48ff3..37066e4d80 100644 --- a/crates/nu-command/src/filters/select.rs +++ b/crates/nu-command/src/filters/select.rs @@ -1,9 +1,9 @@ use nu_engine::CallExt; -use nu_protocol::ast::{Call, CellPath}; +use nu_protocol::ast::{Call, CellPath, PathMember}; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, ShellError, - Signature, Span, SyntaxShape, Value, + Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, + PipelineIterator, ShellError, Signature, Span, SyntaxShape, Value, }; #[derive(Clone)] @@ -14,6 +14,7 @@ impl Command for Select { "select" } + // FIXME: also add support for --skip fn signature(&self) -> Signature { Signature::build("select") .rest( @@ -63,9 +64,44 @@ fn select( columns: Vec, input: PipelineData, ) -> Result { - if columns.is_empty() { - return Err(ShellError::CantFindColumn(span, span)); //FIXME? + let mut rows = vec![]; + + let mut new_columns = vec![]; + + for column in columns { + let CellPath { ref members } = column; + match members.get(0) { + Some(PathMember::Int { val, span }) => { + if members.len() > 1 { + return Err(ShellError::SpannedLabeledError( + "Select only allows row numbers for rows".into(), + "extra after row number".into(), + *span, + )); + } + + rows.push(*val); + } + _ => new_columns.push(column), + }; } + let columns = new_columns; + + let input = if !rows.is_empty() { + rows.sort_unstable(); + // let skip = call.has_flag("skip"); + let pipeline_iter: PipelineIterator = input.into_iter(); + + NthIterator { + input: pipeline_iter, + rows, + skip: false, + current: 0, + } + .into_pipeline_data(engine_state.ctrlc.clone()) + } else { + input + }; match input { PipelineData::Value( @@ -78,17 +114,21 @@ fn select( let mut output = vec![]; for input_val in input_vals { - let mut cols = vec![]; - let mut vals = vec![]; - for path in &columns { - //FIXME: improve implementation to not clone - let fetcher = input_val.clone().follow_cell_path(&path.members)?; + if !columns.is_empty() { + let mut cols = vec![]; + let mut vals = vec![]; + for path in &columns { + //FIXME: improve implementation to not clone + let fetcher = input_val.clone().follow_cell_path(&path.members)?; - cols.push(path.into_string()); - vals.push(fetcher); + cols.push(path.into_string()); + vals.push(fetcher); + } + + output.push(Value::Record { cols, vals, span }) + } else { + output.push(input_val) } - - output.push(Value::Record { cols, vals, span }) } Ok(output @@ -97,39 +137,89 @@ fn select( } PipelineData::ListStream(stream, ..) => Ok(stream .map(move |x| { - let mut cols = vec![]; - let mut vals = vec![]; - for path in &columns { - //FIXME: improve implementation to not clone - match x.clone().follow_cell_path(&path.members) { - Ok(value) => { - cols.push(path.into_string()); - vals.push(value); - } - Err(error) => { - cols.push(path.into_string()); - vals.push(Value::Error { error }); + if !columns.is_empty() { + let mut cols = vec![]; + let mut vals = vec![]; + for path in &columns { + //FIXME: improve implementation to not clone + match x.clone().follow_cell_path(&path.members) { + Ok(value) => { + cols.push(path.into_string()); + vals.push(value); + } + Err(error) => { + cols.push(path.into_string()); + vals.push(Value::Error { error }); + } } } + Value::Record { cols, vals, span } + } else { + x } - - Value::Record { cols, vals, span } }) .into_pipeline_data(engine_state.ctrlc.clone())), PipelineData::Value(v, ..) => { - let mut cols = vec![]; - let mut vals = vec![]; + if !columns.is_empty() { + let mut cols = vec![]; + let mut vals = vec![]; - for cell_path in columns { - // FIXME: remove clone - let result = v.clone().follow_cell_path(&cell_path.members)?; + for cell_path in columns { + // FIXME: remove clone + let result = v.clone().follow_cell_path(&cell_path.members)?; - cols.push(cell_path.into_string()); - vals.push(result); + cols.push(cell_path.into_string()); + vals.push(result); + } + + Ok(Value::Record { cols, vals, span }.into_pipeline_data()) + } else { + Ok(v.into_pipeline_data()) } - - Ok(Value::Record { cols, vals, span }.into_pipeline_data()) } _ => Ok(PipelineData::new(span)), } } + +struct NthIterator { + input: PipelineIterator, + rows: Vec, + skip: bool, + current: usize, +} + +impl Iterator for NthIterator { + type Item = Value; + + fn next(&mut self) -> Option { + loop { + if !self.skip { + if let Some(row) = self.rows.get(0) { + if self.current == *row { + self.rows.remove(0); + self.current += 1; + return self.input.next(); + } else { + self.current += 1; + let _ = self.input.next(); + continue; + } + } else { + return None; + } + } else if let Some(row) = self.rows.get(0) { + if self.current == *row { + self.rows.remove(0); + self.current += 1; + let _ = self.input.next(); + continue; + } else { + self.current += 1; + return self.input.next(); + } + } else { + return self.input.next(); + } + } + } +} diff --git a/crates/nu-command/tests/commands/get.rs b/crates/nu-command/tests/commands/get.rs index 4bd00cf16c..14c6dc1b36 100644 --- a/crates/nu-command/tests/commands/get.rs +++ b/crates/nu-command/tests/commands/get.rs @@ -108,7 +108,7 @@ fn fetches_more_than_one_column_path() { arepas = 1 [[fortune_tellers]] - name = "Jonathan Turner" + name = "JT" arepas = 1 [[fortune_tellers]] @@ -126,7 +126,7 @@ fn fetches_more_than_one_column_path() { "# )); - assert_eq!(actual.out, "Jonathan Turner"); + assert_eq!(actual.out, "JT"); }) } diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs index 985c93afed..8ced100b19 100644 --- a/crates/nu-command/tests/commands/mod.rs +++ b/crates/nu-command/tests/commands/mod.rs @@ -34,7 +34,6 @@ mod math; mod merge; mod mkdir; mod move_; -mod nth; mod open; mod parse; mod path; diff --git a/crates/nu-command/tests/commands/move_/column.rs b/crates/nu-command/tests/commands/move_/column.rs index 610b0390f3..cc807e9f9f 100644 --- a/crates/nu-command/tests/commands/move_/column.rs +++ b/crates/nu-command/tests/commands/move_/column.rs @@ -133,7 +133,7 @@ fn moves_columns_after() { open sample.csv | move letters and_more --after column1 | get - | nth 1 2 + | select 1 2 | str collect "# )); diff --git a/crates/nu-command/tests/commands/nth.rs b/crates/nu-command/tests/commands/nth.rs deleted file mode 100644 index 41af901cdf..0000000000 --- a/crates/nu-command/tests/commands/nth.rs +++ /dev/null @@ -1,41 +0,0 @@ -use nu_test_support::fs::Stub::EmptyFile; -use nu_test_support::playground::Playground; -use nu_test_support::{nu, pipeline}; - -#[test] -fn selects_a_row() { - Playground::setup("nth_test_1", |dirs, sandbox| { - sandbox.with_files(vec![EmptyFile("notes.txt"), EmptyFile("arepas.txt")]); - - let actual = nu!( - cwd: dirs.test(), pipeline( - r#" - ls - | sort-by name - | nth 0 - | get name.0 - "# - )); - - assert_eq!(actual.out, "arepas.txt"); - }); -} - -#[test] -fn selects_many_rows() { - Playground::setup("nth_test_2", |dirs, sandbox| { - sandbox.with_files(vec![EmptyFile("notes.txt"), EmptyFile("arepas.txt")]); - - let actual = nu!( - cwd: dirs.test(), pipeline( - r#" - ls - | get name - | nth 1 0 - | length - "# - )); - - assert_eq!(actual.out, "2"); - }); -} diff --git a/crates/nu-command/tests/commands/open.rs b/crates/nu-command/tests/commands/open.rs index d03f25ff1d..5e5a5d8bb6 100644 --- a/crates/nu-command/tests/commands/open.rs +++ b/crates/nu-command/tests/commands/open.rs @@ -59,7 +59,7 @@ fn parses_csv() { fn parses_bson() { let actual = nu!( cwd: "tests/fixtures/formats", - "open sample.bson | get root | nth 0 | get b" + "open sample.bson | get root | select 0 | get b" ); assert_eq!(actual.out, "hello"); @@ -73,7 +73,7 @@ fn parses_more_bson_complexity() { r#" open sample.bson | get root - | nth 6 + | select 6 | get b | get '$binary_subtype' "# @@ -120,7 +120,7 @@ fn parses_more_bson_complexity() { // `ints` has just `z`, and `floats` has only the column `f`. This means, in general, when working // with sqlite, one will want to select a single table, e.g.: // -// open sample.db | nth 1 | get table_values +// open sample.db | select 1 | get table_values // ━━━┯━━━━━━ // # │ z // ───┼────── @@ -139,7 +139,7 @@ fn parses_sqlite() { r#" open sample.db | get table_values - | nth 2 + | select 2 | get x "# )); diff --git a/crates/nu-command/tests/commands/parse.rs b/crates/nu-command/tests/commands/parse.rs index 82274f9587..f2e6f1b7a3 100644 --- a/crates/nu-command/tests/commands/parse.rs +++ b/crates/nu-command/tests/commands/parse.rs @@ -25,7 +25,7 @@ mod simple { open key_value_separated_arepa_ingredients.txt | lines | each { echo $it | parse "{Name}={Value}" } - | nth 1 + | select 1 | get Value "# )); diff --git a/crates/nu-command/tests/commands/select.rs b/crates/nu-command/tests/commands/select.rs index 7b15c27369..8881782294 100644 --- a/crates/nu-command/tests/commands/select.rs +++ b/crates/nu-command/tests/commands/select.rs @@ -1,4 +1,4 @@ -use nu_test_support::fs::Stub::FileWithContentToBeTrimmed; +use nu_test_support::fs::Stub::{EmptyFile, FileWithContentToBeTrimmed}; use nu_test_support::playground::Playground; use nu_test_support::{nu, pipeline}; @@ -126,3 +126,41 @@ fn ignores_duplicate_columns_selected() { assert_eq!(actual.out, "first name last name"); } + +#[test] +fn selects_a_row() { + Playground::setup("select_test_1", |dirs, sandbox| { + sandbox.with_files(vec![EmptyFile("notes.txt"), EmptyFile("arepas.txt")]); + + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + ls + | sort-by name + | select 0 + | get name.0 + "# + )); + + assert_eq!(actual.out, "arepas.txt"); + }); +} + +#[test] +fn selects_many_rows() { + Playground::setup("select_test_2", |dirs, sandbox| { + sandbox.with_files(vec![EmptyFile("notes.txt"), EmptyFile("arepas.txt")]); + + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + ls + | get name + | select 1 0 + | length + "# + )); + + assert_eq!(actual.out, "2"); + }); +} diff --git a/crates/nu-command/tests/format_conversions/sqlite.rs b/crates/nu-command/tests/format_conversions/sqlite.rs index de6ce8ccfa..338d876c61 100644 --- a/crates/nu-command/tests/format_conversions/sqlite.rs +++ b/crates/nu-command/tests/format_conversions/sqlite.rs @@ -11,7 +11,7 @@ fn table_to_sqlite_and_back_into_table() { | to sqlite | from sqlite | get table_values - | nth 2 + | select 2 | get x "# )); diff --git a/crates/nu-command/tests/format_conversions/tsv.rs b/crates/nu-command/tests/format_conversions/tsv.rs index 066fa16ad3..e81a3c846b 100644 --- a/crates/nu-command/tests/format_conversions/tsv.rs +++ b/crates/nu-command/tests/format_conversions/tsv.rs @@ -43,7 +43,7 @@ fn table_to_tsv_text() { | last 1 | to tsv | lines - | nth 1 + | select 1 "# )); diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 29e6a4653e..eb85bf4a2c 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -564,7 +564,7 @@ impl Value { Value::Nothing { span } } - /// Follow a given column path into the value: for example accessing nth elements in a stream or list + /// Follow a given column path into the value: for example accessing select elements in a stream or list pub fn follow_cell_path(self, cell_path: &[PathMember]) -> Result { let mut current = self; for member in cell_path { @@ -575,7 +575,7 @@ impl Value { val: count, span: origin_span, } => { - // Treat a numeric path member as `nth ` + // Treat a numeric path member as `select ` match &mut current { Value::List { vals: val, .. } => { if let Some(item) = val.get(*count) { @@ -671,7 +671,7 @@ impl Value { Ok(current) } - /// Follow a given column path into the value: for example accessing nth elements in a stream or list + /// Follow a given column path into the value: for example accessing select elements in a stream or list pub fn update_cell_path( &mut self, cell_path: &[PathMember], diff --git a/docs/commands/lines.md b/docs/commands/lines.md index dba293e0c8..3718c5be9d 100644 --- a/docs/commands/lines.md +++ b/docs/commands/lines.md @@ -26,7 +26,7 @@ Basic usage: One useful application is piping the contents of file into `lines`. This example extracts a certain line from a given file. ```shell -> cat lines.md | lines | nth 6 +> cat lines.md | lines | select 6 ## Examples ``` diff --git a/docs/commands/nth.md b/docs/commands/nth.md index 05cbfbb05c..1e221fec87 100644 --- a/docs/commands/nth.md +++ b/docs/commands/nth.md @@ -1,12 +1,12 @@ # nth -This command returns the nth row of a table, starting from 0. +This command returns the select row of a table, starting from 0. If the number given is less than 0 or more than the number of rows, nothing is returned. ## Usage ```shell -> [input-command] | nth ...args +> [input-command] | select ...args ``` ## Parameters @@ -45,7 +45,7 @@ If the number given is less than 0 or more than the number of rows, nothing is r ``` ```shell -> ls | nth 0 +> ls | select 0 ──────────┬──────────────────── name │ CODE_OF_CONDUCT.md type │ File @@ -55,7 +55,7 @@ If the number given is less than 0 or more than the number of rows, nothing is r ``` ```shell -> ls | nth 0 2 +> ls | select 0 2 ───┬────────────────────┬──────┬──────────┬───────────── # │ name │ type │ size │ modified ───┼────────────────────┼──────┼──────────┼───────────── @@ -65,7 +65,7 @@ If the number given is less than 0 or more than the number of rows, nothing is r ``` ```shell -> ls | nth 5 +> ls | select 5 ──────────┬─────────────── name │ Makefile.toml type │ File