From e81689f2c0ca8e1d2e3b208cfcb4418692acea80 Mon Sep 17 00:00:00 2001 From: pwygab <88221256+merelymyself@users.noreply.github.com> Date: Sat, 3 Sep 2022 20:35:36 +0800 Subject: [PATCH] Allow for rejecting nested record cells (#6463) * add new function to remove data at a cellpath; allow reject to use cellpath * add tests * fmt * fix clippt * get it working properly with lists of records * fix clippy, hopefully * fix clippy, hopefully 2 --- crates/nu-command/src/filters/reject.rs | 169 ++++----------------- crates/nu-command/tests/commands/reject.rs | 13 ++ crates/nu-protocol/src/value/mod.rs | 135 ++++++++++++++++ 3 files changed, 179 insertions(+), 138 deletions(-) diff --git a/crates/nu-command/src/filters/reject.rs b/crates/nu-command/src/filters/reject.rs index f86818a181..a7029638c0 100644 --- a/crates/nu-command/src/filters/reject.rs +++ b/crates/nu-command/src/filters/reject.rs @@ -2,8 +2,8 @@ use nu_engine::CallExt; use nu_protocol::ast::{Call, CellPath}; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Example, FromValue, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, - ShellError, Signature, Span, SyntaxShape, Value, + Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, SyntaxShape, + Value, }; #[derive(Clone)] @@ -18,7 +18,7 @@ impl Command for Reject { Signature::build("reject") .rest( "rest", - SyntaxShape::String, + SyntaxShape::CellPath, "the names of columns to remove from the table", ) .category(Category::Filters) @@ -35,7 +35,7 @@ impl Command for Reject { call: &Call, input: PipelineData, ) -> Result { - let columns: Vec = call.rest(engine_state, stack, 0)?; + let columns: Vec = call.rest(engine_state, stack, 0)?; let span = call.head; reject(engine_state, span, input, columns) } @@ -59,151 +59,44 @@ impl Command for Reject { span: Span::test_data(), }), }, + Example { + description: "Reject a nested field in a record", + example: "echo {a: {b: 3,c: 5}} | reject a.b", + result: Some(Value::Record { + cols: vec!["a".into()], + vals: vec![Value::Record { + cols: vec!["c".into()], + vals: vec![Value::Int { + val: 5, + span: Span::test_data(), + }], + span: Span::test_data(), + }], + span: Span::test_data(), + }), + }, ] } } fn reject( - engine_state: &EngineState, + _engine_state: &EngineState, span: Span, input: PipelineData, - columns: Vec, + cell_paths: Vec, ) -> Result { - if columns.is_empty() { - return Err(ShellError::CantFindColumn(span, span)); - } - let metadata = input.metadata(); - - let mut keep_columns = vec![]; - - match input { - PipelineData::Value( - Value::List { - vals: input_vals, - span, - }, - .., - ) => { - let mut output = vec![]; - let input_cols = get_input_cols(input_vals.clone()); - let kc = get_keep_columns(input_cols, columns); - keep_columns = get_cellpath_columns(kc, span); - - for input_val in input_vals { - let mut cols = vec![]; - let mut vals = vec![]; - - for path in &keep_columns { - let fetcher = input_val.clone().follow_cell_path(&path.members, false); - - if let Ok(value) = fetcher { - cols.push(path.into_string()); - vals.push(value); - } - } - output.push(Value::Record { cols, vals, span }) - } - - Ok(output - .into_iter() - .into_pipeline_data(engine_state.ctrlc.clone())) - } - PipelineData::Value( - Value::Record { - mut cols, - mut vals, - span, - }, - metadata, - ) => { - reject_record_columns(&mut cols, &mut vals, &columns); - - let record = Value::Record { cols, vals, span }; - - Ok(PipelineData::Value(record, metadata)) - } - PipelineData::ListStream(stream, ..) => { - let mut output = vec![]; - - let v: Vec<_> = stream.into_iter().collect(); - let input_cols = get_input_cols(v.clone()); - let kc = get_keep_columns(input_cols, columns); - keep_columns = get_cellpath_columns(kc, span); - - for input_val in v { - let mut cols = vec![]; - let mut vals = vec![]; - - for path in &keep_columns { - let fetcher = input_val.clone().follow_cell_path(&path.members, false)?; - cols.push(path.into_string()); - vals.push(fetcher); - } - output.push(Value::Record { cols, vals, span }) - } - - Ok(output - .into_iter() - .into_pipeline_data(engine_state.ctrlc.clone())) - } - PipelineData::Value(v, ..) => { - let mut cols = vec![]; - let mut vals = vec![]; - - for cell_path in &keep_columns { - let result = v.clone().follow_cell_path(&cell_path.members, false)?; - - cols.push(cell_path.into_string()); - vals.push(result); - } - - Ok(Value::Record { cols, vals, span }.into_pipeline_data()) - } - x => Ok(x), - } - .map(|x| x.set_metadata(metadata)) -} - -fn get_input_cols(input: Vec) -> Vec { - let rec = input.first(); - match rec { - Some(Value::Record { cols, vals: _, .. }) => cols.to_vec(), - _ => vec!["".to_string()], - } -} - -fn get_cellpath_columns(keep_cols: Vec, span: Span) -> Vec { - let mut output = vec![]; - for keep_col in keep_cols { - let val = Value::String { - val: keep_col, - span, - }; - let cell_path = match CellPath::from_value(&val) { - Ok(v) => v, - Err(_) => return vec![], - }; - output.push(cell_path); - } - output -} - -fn get_keep_columns(mut input: Vec, rejects: Vec) -> Vec { - for reject in rejects { - if let Some(index) = input.iter().position(|value| *value == reject) { - input.remove(index); + let val = input.into_value(span); + let mut val = val; + let mut columns = vec![]; + for c in cell_paths { + if !columns.contains(&c) { + columns.push(c); } } - input -} - -fn reject_record_columns(cols: &mut Vec, vals: &mut Vec, rejects: &[String]) { - for reject in rejects { - if let Some(index) = cols.iter().position(|value| value == reject) { - cols.remove(index); - vals.remove(index); - } + for cell_path in columns { + val.remove_data_at_cell_path(&cell_path.members)?; } + Ok(val.into_pipeline_data()) } #[cfg(test)] diff --git a/crates/nu-command/tests/commands/reject.rs b/crates/nu-command/tests/commands/reject.rs index df8c00ac7f..9060360a0e 100644 --- a/crates/nu-command/tests/commands/reject.rs +++ b/crates/nu-command/tests/commands/reject.rs @@ -107,3 +107,16 @@ fn reject_table_from_raw_eval() { assert!(actual.out.contains("record 0 fields")); } + +#[test] +fn reject_nested_field() { + let actual = nu!( + cwd: ".", pipeline( + r#" + {a:{b:3,c:5}} | reject a.b | debug + "# + ) + ); + + assert_eq!(actual.out, "{a: {c: 5}}"); +} diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index b8c9b698b5..c07bf4ca7e 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -926,6 +926,141 @@ impl Value { Ok(()) } + pub fn remove_data_at_cell_path(&mut self, cell_path: &[PathMember]) -> Result<(), ShellError> { + match cell_path.len() { + 0 => Ok(()), + 1 => { + let path_member = cell_path.first().expect("there is a first"); + match path_member { + PathMember::String { + val: col_name, + span, + } => match self { + Value::List { vals, .. } => { + for val in vals.iter_mut() { + match val { + Value::Record { + cols, + vals, + span: v_span, + } => { + let mut found = false; + for (i, col) in cols.clone().iter().enumerate() { + if col == col_name { + cols.remove(i); + vals.remove(i); + found = true; + } + } + if !found { + return Err(ShellError::CantFindColumn(*span, *v_span)); + } + } + v => return Err(ShellError::CantFindColumn(*span, v.span()?)), + } + } + Ok(()) + } + Value::Record { + cols, + vals, + span: v_span, + } => { + let mut found = false; + for (i, col) in cols.clone().iter().enumerate() { + if col == col_name { + cols.remove(i); + vals.remove(i); + found = true; + } + } + if !found { + return Err(ShellError::CantFindColumn(*span, *v_span)); + } + Ok(()) + } + v => Err(ShellError::CantFindColumn(*span, v.span()?)), + }, + PathMember::Int { val: row_num, span } => match self { + Value::List { vals, .. } => { + if vals.get_mut(*row_num).is_some() { + vals.remove(*row_num); + Ok(()) + } else { + Err(ShellError::AccessBeyondEnd(vals.len(), *span)) + } + } + v => Err(ShellError::NotAList(*span, v.span()?)), + }, + } + } + _ => { + let path_member = cell_path.first().expect("there is a first"); + match path_member { + PathMember::String { + val: col_name, + span, + } => match self { + Value::List { vals, .. } => { + for val in vals.iter_mut() { + match val { + Value::Record { + cols, + vals, + span: v_span, + } => { + let mut found = false; + for col in cols.iter().zip(vals.iter_mut()) { + if col.0 == col_name { + found = true; + col.1.remove_data_at_cell_path(&cell_path[1..])? + } + } + if !found { + return Err(ShellError::CantFindColumn(*span, *v_span)); + } + } + v => return Err(ShellError::CantFindColumn(*span, v.span()?)), + } + } + Ok(()) + } + Value::Record { + cols, + vals, + span: v_span, + } => { + let mut found = false; + + for col in cols.iter().zip(vals.iter_mut()) { + if col.0 == col_name { + found = true; + + col.1.remove_data_at_cell_path(&cell_path[1..])? + } + } + if !found { + return Err(ShellError::CantFindColumn(*span, *v_span)); + } + Ok(()) + } + v => Err(ShellError::CantFindColumn(*span, v.span()?)), + }, + PathMember::Int { val: row_num, span } => match self { + Value::List { vals, .. } => { + if let Some(v) = vals.get_mut(*row_num) { + v.remove_data_at_cell_path(&cell_path[1..]) + } else { + Err(ShellError::AccessBeyondEnd(vals.len(), *span)) + } + } + v => Err(ShellError::NotAList(*span, v.span()?)), + }, + } + } + } + } + pub fn insert_data_at_cell_path( &mut self, cell_path: &[PathMember],