diff --git a/crates/nu-command/src/filters/insert.rs b/crates/nu-command/src/filters/insert.rs index b07383121e..fd08665ed9 100644 --- a/crates/nu-command/src/filters/insert.rs +++ b/crates/nu-command/src/filters/insert.rs @@ -1,9 +1,9 @@ use nu_engine::{eval_block, CallExt}; -use nu_protocol::ast::{Call, CellPath, PathMember}; +use nu_protocol::ast::{Block, Call, CellPath, PathMember}; use nu_protocol::engine::{Closure, Command, EngineState, Stack}; use nu_protocol::{ record, Category, Example, FromValue, IntoInterruptiblePipelineData, IntoPipelineData, - PipelineData, ShellError, Signature, SyntaxShape, Type, Value, + PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, }; #[derive(Clone)] @@ -57,49 +57,64 @@ impl Command for Insert { } fn examples(&self) -> Vec { - vec![Example { - description: "Insert a new entry into a single record", - example: "{'name': 'nu', 'stars': 5} | insert alias 'Nushell'", - result: Some(Value::test_record(record! { - "name" => Value::test_string("nu"), - "stars" => Value::test_int(5), - "alias" => Value::test_string("Nushell"), - })), - }, - Example { - description: "Insert a new column into a table, populating all rows", - example: "[[project, lang]; ['Nushell', 'Rust']] | insert type 'shell'", - result: Some(Value::test_list ( - vec![Value::test_record(record! { + vec![ + Example { + description: "Insert a new entry into a single record", + example: "{'name': 'nu', 'stars': 5} | insert alias 'Nushell'", + result: Some(Value::test_record(record! { + "name" => Value::test_string("nu"), + "stars" => Value::test_int(5), + "alias" => Value::test_string("Nushell"), + })), + }, + Example { + description: "Insert a new column into a table, populating all rows", + example: "[[project, lang]; ['Nushell', 'Rust']] | insert type 'shell'", + result: Some(Value::test_list(vec![Value::test_record(record! { "project" => Value::test_string("Nushell"), "lang" => Value::test_string("Rust"), "type" => Value::test_string("shell"), - })], - )), - }, - Example { - description: "Insert a column with values equal to their row index, plus the value of 'foo' in each row", - example: "[[foo]; [7] [8] [9]] | enumerate | insert bar {|e| $e.item.foo + $e.index } | flatten", - result: Some(Value::test_list ( - vec![ + })])), + }, + Example { + description: "Insert a new column with values computed based off the other columns", + example: "[[foo]; [7] [8] [9]] | insert bar {|row| $row.foo * 2 }", + result: Some(Value::test_list(vec![ Value::test_record(record! { - "index" => Value::test_int(0), - "foo" => Value::test_int(7), - "bar" => Value::test_int(7), + "foo" => Value::test_int(7), + "bar" => Value::test_int(14), }), Value::test_record(record! { - "index" => Value::test_int(1), - "foo" => Value::test_int(8), - "bar" => Value::test_int(9), + "foo" => Value::test_int(8), + "bar" => Value::test_int(16), }), Value::test_record(record! { - "index" => Value::test_int(2), - "foo" => Value::test_int(9), - "bar" => Value::test_int(11), + "foo" => Value::test_int(9), + "bar" => Value::test_int(18), }), - ], - )), - }] + ])), + }, + Example { + description: "Insert a new value into a list at an index", + example: "[1 2 4] | insert 2 3", + result: Some(Value::test_list(vec![ + Value::test_int(1), + Value::test_int(2), + Value::test_int(3), + Value::test_int(4), + ])), + }, + Example { + description: "Insert a new value at the end of a list", + example: "[1 2 3] | insert 3 4", + result: Some(Value::test_list(vec![ + Value::test_int(1), + Value::test_int(2), + Value::test_int(3), + Value::test_int(4), + ])), + }, + ] } } @@ -109,7 +124,6 @@ fn insert( call: &Call, input: PipelineData, ) -> Result { - let metadata = input.metadata(); let span = call.head; let cell_path: CellPath = call.req(engine_state, stack, 0)?; @@ -118,99 +132,263 @@ fn insert( let redirect_stdout = call.redirect_stdout; let redirect_stderr = call.redirect_stderr; - let engine_state = engine_state.clone(); let ctrlc = engine_state.ctrlc.clone(); - // Replace is a block, so set it up and run it instead of using it as the replacement - if replacement.as_block().is_ok() { - let capture_block = Closure::from_value(replacement)?; - let block = engine_state.get_block(capture_block.block_id).clone(); - - let mut stack = stack.captures_to_stack(capture_block.captures); - let orig_env_vars = stack.env_vars.clone(); - let orig_env_hidden = stack.env_hidden.clone(); - - input - .map( - move |mut input| { - // with_env() is used here to ensure that each iteration uses - // a different set of environment variables. - // Hence, a 'cd' in the first loop won't affect the next loop. - stack.with_env(&orig_env_vars, &orig_env_hidden); - - // Element argument - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, input.clone()) + match input { + PipelineData::Value(mut value, metadata) => { + if replacement.as_block().is_ok() { + match (cell_path.members.first(), &mut value) { + (Some(PathMember::String { .. }), Value::List { vals, .. }) => { + let span = replacement.span(); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id); + let stack = stack.captures_to_stack(capture_block.captures.clone()); + for val in vals { + let mut stack = stack.clone(); + insert_value_by_closure( + val, + span, + engine_state, + &mut stack, + redirect_stdout, + redirect_stderr, + block, + &cell_path.members, + false, + )?; } } + (first, _) => { + insert_single_value_by_closure( + &mut value, + replacement, + engine_state, + stack, + redirect_stdout, + redirect_stderr, + &cell_path.members, + matches!(first, Some(PathMember::Int { .. })), + )?; + } + } + } else { + value.insert_data_at_cell_path(&cell_path.members, replacement, span)?; + } + Ok(value.into_pipeline_data_with_metadata(metadata)) + } + PipelineData::ListStream(mut stream, metadata) => { + if let Some(( + &PathMember::Int { + val, + span: path_span, + .. + }, + path, + )) = cell_path.members.split_first() + { + let mut pre_elems = vec![]; - let output = eval_block( - &engine_state, - &mut stack, - &block, - input.clone().into_pipeline_data(), - redirect_stdout, - redirect_stderr, - ); + for idx in 0..val { + if let Some(v) = stream.next() { + pre_elems.push(v); + } else { + return Err(ShellError::InsertAfterNextFreeIndex { + available_idx: idx, + span: path_span, + }); + } + } - match output { - Ok(pd) => { - let span = pd.span().unwrap_or(span); - if let Err(e) = input.insert_data_at_cell_path( - &cell_path.members, - pd.into_value(span), - span, - ) { - return Value::error(e, span); + if path.is_empty() { + if replacement.as_block().is_ok() { + let span = replacement.span(); + let value = stream.next(); + let end_of_stream = value.is_none(); + let value = value.unwrap_or(Value::nothing(span)); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id); + let mut stack = stack.captures_to_stack(capture_block.captures); + + if let Some(var) = block.signature.get_positional(0) { + if let Some(var_id) = &var.var_id { + stack.add_var(*var_id, value.clone()) } + } + let output = eval_block( + engine_state, + &mut stack, + block, + value.clone().into_pipeline_data(), + redirect_stdout, + redirect_stderr, + )?; + + pre_elems.push(output.into_value(span)); + if !end_of_stream { + pre_elems.push(value); + } + } else { + pre_elems.push(replacement); + } + } else if let Some(mut value) = stream.next() { + if replacement.as_block().is_ok() { + insert_single_value_by_closure( + &mut value, + replacement, + engine_state, + stack, + redirect_stdout, + redirect_stderr, + path, + true, + )?; + } else { + value.insert_data_at_cell_path(path, replacement, span)?; + } + pre_elems.push(value) + } else { + return Err(ShellError::AccessBeyondEnd { + max_idx: pre_elems.len() - 1, + span: path_span, + }); + } + + Ok(pre_elems + .into_iter() + .chain(stream) + .into_pipeline_data_with_metadata(metadata, ctrlc)) + } else if replacement.as_block().is_ok() { + let engine_state = engine_state.clone(); + let replacement_span = replacement.span(); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id).clone(); + let stack = stack.captures_to_stack(capture_block.captures.clone()); + + Ok(stream + .map(move |mut input| { + // Recreate the stack for each iteration to + // isolate environment variable changes, etc. + let mut stack = stack.clone(); + + let err = insert_value_by_closure( + &mut input, + replacement_span, + &engine_state, + &mut stack, + redirect_stdout, + redirect_stderr, + &block, + &cell_path.members, + false, + ); + + if let Err(e) = err { + Value::error(e, span) + } else { input } - Err(e) => Value::error(e, span), - } - }, - ctrlc, - ) - .map(|x| x.set_metadata(metadata)) - } else { - if let Some(PathMember::Int { val, .. }) = cell_path.members.first() { - let mut input = input.into_iter(); - let mut pre_elems = vec![]; - - for _ in 0..*val { - if let Some(v) = input.next() { - pre_elems.push(v); - } else { - pre_elems.push(Value::nothing(span)) - } + }) + .into_pipeline_data_with_metadata(metadata, ctrlc)) + } else { + Ok(stream + .map(move |mut input| { + if let Err(e) = input.insert_data_at_cell_path( + &cell_path.members, + replacement.clone(), + span, + ) { + Value::error(e, span) + } else { + input + } + }) + .into_pipeline_data_with_metadata(metadata, ctrlc)) } - - return Ok(pre_elems - .into_iter() - .chain(vec![replacement]) - .chain(input) - .into_pipeline_data_with_metadata(metadata, ctrlc)); } - input - .map( - move |mut input| { - let replacement = replacement.clone(); - - if let Err(e) = - input.insert_data_at_cell_path(&cell_path.members, replacement, span) - { - return Value::error(e, span); - } - - input - }, - ctrlc, - ) - .map(|x| x.set_metadata(metadata)) + PipelineData::Empty => Err(ShellError::IncompatiblePathAccess { + type_name: "empty pipeline".to_string(), + span, + }), + PipelineData::ExternalStream { .. } => Err(ShellError::IncompatiblePathAccess { + type_name: "external stream".to_string(), + span, + }), } } +#[allow(clippy::too_many_arguments)] +fn insert_value_by_closure( + value: &mut Value, + span: Span, + engine_state: &EngineState, + stack: &mut Stack, + redirect_stdout: bool, + redirect_stderr: bool, + block: &Block, + cell_path: &[PathMember], + first_path_member_int: bool, +) -> Result<(), ShellError> { + let input_at_path = value.clone().follow_cell_path(cell_path, false); + + if let Some(var) = block.signature.get_positional(0) { + if let Some(var_id) = &var.var_id { + stack.add_var( + *var_id, + if first_path_member_int { + input_at_path.clone().unwrap_or(Value::nothing(span)) + } else { + value.clone() + }, + ) + } + } + + let input_at_path = input_at_path + .map(IntoPipelineData::into_pipeline_data) + .unwrap_or(PipelineData::Empty); + + let output = eval_block( + engine_state, + stack, + block, + input_at_path, + redirect_stdout, + redirect_stderr, + )?; + + value.insert_data_at_cell_path(cell_path, output.into_value(span), span) +} + +#[allow(clippy::too_many_arguments)] +fn insert_single_value_by_closure( + value: &mut Value, + replacement: Value, + engine_state: &EngineState, + stack: &mut Stack, + redirect_stdout: bool, + redirect_stderr: bool, + cell_path: &[PathMember], + first_path_member_int: bool, +) -> Result<(), ShellError> { + let span = replacement.span(); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id); + let mut stack = stack.captures_to_stack(capture_block.captures); + + insert_value_by_closure( + value, + span, + engine_state, + &mut stack, + redirect_stdout, + redirect_stderr, + block, + cell_path, + first_path_member_int, + ) +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/nu-command/src/filters/update.rs b/crates/nu-command/src/filters/update.rs index 5dd4b54d06..7b46e40a42 100644 --- a/crates/nu-command/src/filters/update.rs +++ b/crates/nu-command/src/filters/update.rs @@ -1,9 +1,9 @@ use nu_engine::{eval_block, CallExt}; -use nu_protocol::ast::{Call, CellPath, PathMember}; +use nu_protocol::ast::{Block, Call, CellPath, PathMember}; use nu_protocol::engine::{Closure, Command, EngineState, Stack}; use nu_protocol::{ record, Category, Example, FromValue, IntoInterruptiblePipelineData, IntoPipelineData, - PipelineData, ShellError, Signature, SyntaxShape, Type, Value, + PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, }; #[derive(Clone)] @@ -63,18 +63,8 @@ impl Command for Update { })), }, Example { - description: "Use in closure form for more involved updating logic", - example: "[[count fruit]; [1 'apple']] | enumerate | update item.count {|e| ($e.item.fruit | str length) + $e.index } | get item", - result: Some(Value::test_list( - vec![Value::test_record(record! { - "count" => Value::test_int(5), - "fruit" => Value::test_string("apple"), - })], - )), - }, - Example { - description: "Alter each value in the 'authors' column to use a single string instead of a list", - example: "[[project, authors]; ['nu', ['Andrés', 'JT', 'Yehuda']]] | update authors {|row| $row.authors | str join ','}", + description: "Use a closure to alter each value in the 'authors' column to a single string", + example: "[[project, authors]; ['nu', ['Andrés', 'JT', 'Yehuda']]] | update authors {|row| $row.authors | str join ',' }", result: Some(Value::test_list( vec![Value::test_record(record! { "project" => Value::test_string("nu"), @@ -84,14 +74,28 @@ impl Command for Update { }, Example { description: "You can also use a simple command to update 'authors' to a single string", - example: "[[project, authors]; ['nu', ['Andrés', 'JT', 'Yehuda']]] | update authors {|| str join ','}", + example: "[[project, authors]; ['nu', ['Andrés', 'JT', 'Yehuda']]] | update authors { str join ',' }", result: Some(Value::test_list( vec![Value::test_record(record! { "project" => Value::test_string("nu"), "authors" => Value::test_string("Andrés,JT,Yehuda"), })], )), - } + }, + Example { + description: "Update a value at an index in a list", + example: "[1 2 3] | update 1 4", + result: Some(Value::test_list( + vec![Value::test_int(1), Value::test_int(4), Value::test_int(3)] + )), + }, + Example { + description: "Use a closure to compute a new value at an index", + example: "[1 2 3] | update 1 {|i| $i + 2 }", + result: Some(Value::test_list( + vec![Value::test_int(1), Value::test_int(4), Value::test_int(3)] + )), + }, ] } } @@ -110,111 +114,222 @@ fn update( let redirect_stdout = call.redirect_stdout; let redirect_stderr = call.redirect_stderr; - let engine_state = engine_state.clone(); let ctrlc = engine_state.ctrlc.clone(); - // Let's capture the metadata for ls_colors - let metadata = input.metadata(); - let mdclone = metadata.clone(); - - // Replace is a block, so set it up and run it instead of using it as the replacement - if replacement.as_block().is_ok() { - let capture_block = Closure::from_value(replacement)?; - let block = engine_state.get_block(capture_block.block_id).clone(); - - let mut stack = stack.captures_to_stack(capture_block.captures); - let orig_env_vars = stack.env_vars.clone(); - let orig_env_hidden = stack.env_hidden.clone(); - - Ok(input - .map( - move |mut input| { - // with_env() is used here to ensure that each iteration uses - // a different set of environment variables. - // Hence, a 'cd' in the first loop won't affect the next loop. - stack.with_env(&orig_env_vars, &orig_env_hidden); - - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, input.clone()) + match input { + PipelineData::Value(mut value, metadata) => { + if replacement.as_block().is_ok() { + match (cell_path.members.first(), &mut value) { + (Some(PathMember::String { .. }), Value::List { vals, .. }) => { + let span = replacement.span(); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id); + let stack = stack.captures_to_stack(capture_block.captures.clone()); + for val in vals { + let mut stack = stack.clone(); + update_value_by_closure( + val, + span, + engine_state, + &mut stack, + redirect_stdout, + redirect_stderr, + block, + &cell_path.members, + false, + )?; } } + (first, _) => { + update_single_value_by_closure( + &mut value, + replacement, + engine_state, + stack, + redirect_stdout, + redirect_stderr, + &cell_path.members, + matches!(first, Some(PathMember::Int { .. })), + )?; + } + } + } else { + value.update_data_at_cell_path(&cell_path.members, replacement)?; + } + Ok(value.into_pipeline_data_with_metadata(metadata)) + } + PipelineData::ListStream(mut stream, metadata) => { + if let Some(( + &PathMember::Int { + val, + span: path_span, + .. + }, + path, + )) = cell_path.members.split_first() + { + let mut pre_elems = vec![]; - let input_at_path = - match input.clone().follow_cell_path(&cell_path.members, false) { - Err(e) => return Value::error(e, span), - Ok(v) => v, - }; - let output = eval_block( - &engine_state, - &mut stack, - &block, - input_at_path.into_pipeline_data_with_metadata(metadata.clone()), + for idx in 0..=val { + if let Some(v) = stream.next() { + pre_elems.push(v); + } else if idx == 0 { + return Err(ShellError::AccessEmptyContent { span: path_span }); + } else { + return Err(ShellError::AccessBeyondEnd { + max_idx: idx - 1, + span: path_span, + }); + } + } + + // cannot fail since loop above does at least one iteration or returns an error + let value = pre_elems.last_mut().expect("one element"); + + if replacement.as_block().is_ok() { + update_single_value_by_closure( + value, + replacement, + engine_state, + stack, redirect_stdout, redirect_stderr, - ); + path, + true, + )?; + } else { + value.update_data_at_cell_path(path, replacement)?; + } - match output { - Ok(pd) => { - if let Err(e) = input - .update_data_at_cell_path(&cell_path.members, pd.into_value(span)) - { - return Value::error(e, span); - } + Ok(pre_elems + .into_iter() + .chain(stream) + .into_pipeline_data_with_metadata(metadata, ctrlc)) + } else if replacement.as_block().is_ok() { + let replacement_span = replacement.span(); + let engine_state = engine_state.clone(); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id).clone(); + let stack = stack.captures_to_stack(capture_block.captures.clone()); + Ok(stream + .map(move |mut input| { + // Recreate the stack for each iteration to + // isolate environment variable changes, etc. + let mut stack = stack.clone(); + + let err = update_value_by_closure( + &mut input, + replacement_span, + &engine_state, + &mut stack, + redirect_stdout, + redirect_stderr, + &block, + &cell_path.members, + false, + ); + + if let Err(e) = err { + Value::error(e, span) + } else { input } - Err(e) => Value::error(e, span), - } - }, - ctrlc, - )? - .set_metadata(mdclone)) - } else { - if let Some(PathMember::Int { val, span, .. }) = cell_path.members.first() { - let mut input = input.into_iter(); - let mut pre_elems = vec![]; - - for idx in 0..*val { - if let Some(v) = input.next() { - pre_elems.push(v); - } else if idx == 0 { - return Err(ShellError::AccessEmptyContent { span: *span }); - } else { - return Err(ShellError::AccessBeyondEnd { - max_idx: idx - 1, - span: *span, - }); - } + }) + .into_pipeline_data_with_metadata(metadata, ctrlc)) + } else { + Ok(stream + .map(move |mut input| { + if let Err(e) = + input.update_data_at_cell_path(&cell_path.members, replacement.clone()) + { + Value::error(e, span) + } else { + input + } + }) + .into_pipeline_data_with_metadata(metadata, ctrlc)) } - - // Skip over the replaced value - let _ = input.next(); - - return Ok(pre_elems - .into_iter() - .chain(vec![replacement]) - .chain(input) - .into_pipeline_data_with_metadata(metadata, ctrlc)); } - Ok(input - .map( - move |mut input| { - let replacement = replacement.clone(); - - if let Err(e) = input.update_data_at_cell_path(&cell_path.members, replacement) - { - return Value::error(e, span); - } - - input - }, - ctrlc, - )? - .set_metadata(metadata)) + PipelineData::Empty => Err(ShellError::IncompatiblePathAccess { + type_name: "empty pipeline".to_string(), + span, + }), + PipelineData::ExternalStream { .. } => Err(ShellError::IncompatiblePathAccess { + type_name: "external stream".to_string(), + span, + }), } } +#[allow(clippy::too_many_arguments)] +fn update_value_by_closure( + value: &mut Value, + span: Span, + engine_state: &EngineState, + stack: &mut Stack, + redirect_stdout: bool, + redirect_stderr: bool, + block: &Block, + cell_path: &[PathMember], + first_path_member_int: bool, +) -> Result<(), ShellError> { + let input_at_path = value.clone().follow_cell_path(cell_path, false)?; + + if let Some(var) = block.signature.get_positional(0) { + if let Some(var_id) = &var.var_id { + stack.add_var( + *var_id, + if first_path_member_int { + input_at_path.clone() + } else { + value.clone() + }, + ) + } + } + + let output = eval_block( + engine_state, + stack, + block, + input_at_path.into_pipeline_data(), + redirect_stdout, + redirect_stderr, + )?; + + value.update_data_at_cell_path(cell_path, output.into_value(span)) +} + +#[allow(clippy::too_many_arguments)] +fn update_single_value_by_closure( + value: &mut Value, + replacement: Value, + engine_state: &EngineState, + stack: &mut Stack, + redirect_stdout: bool, + redirect_stderr: bool, + cell_path: &[PathMember], + first_path_member_int: bool, +) -> Result<(), ShellError> { + let span = replacement.span(); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id); + let mut stack = stack.captures_to_stack(capture_block.captures); + + update_value_by_closure( + value, + span, + engine_state, + &mut stack, + redirect_stdout, + redirect_stderr, + block, + cell_path, + first_path_member_int, + ) +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/nu-command/src/filters/upsert.rs b/crates/nu-command/src/filters/upsert.rs index a3e287dff7..f2290c4455 100644 --- a/crates/nu-command/src/filters/upsert.rs +++ b/crates/nu-command/src/filters/upsert.rs @@ -1,9 +1,9 @@ use nu_engine::{eval_block, CallExt}; -use nu_protocol::ast::{Call, CellPath, PathMember}; +use nu_protocol::ast::{Block, Call, CellPath, PathMember}; use nu_protocol::engine::{Closure, Command, EngineState, Stack}; use nu_protocol::{ record, Category, Example, FromValue, IntoInterruptiblePipelineData, IntoPipelineData, - PipelineData, ShellError, Signature, SyntaxShape, Type, Value, + PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, }; #[derive(Clone)] @@ -57,19 +57,28 @@ impl Command for Upsert { } fn examples(&self) -> Vec { - vec![Example { - description: "Update a record's value", - example: "{'name': 'nu', 'stars': 5} | upsert name 'Nushell'", - result: Some(Value::test_record(record! { - "name" => Value::test_string("Nushell"), - "stars" => Value::test_int(5), - })), - }, - Example { - description: "Update each row of a table", - example: "[[name lang]; [Nushell ''] [Reedline '']] | upsert lang 'Rust'", - result: Some(Value::test_list( - vec![ + vec![ + Example { + description: "Update a record's value", + example: "{'name': 'nu', 'stars': 5} | upsert name 'Nushell'", + result: Some(Value::test_record(record! { + "name" => Value::test_string("Nushell"), + "stars" => Value::test_int(5), + })), + }, + Example { + description: "Insert a new entry into a record", + example: "{'name': 'nu', 'stars': 5} | upsert language 'Rust'", + result: Some(Value::test_record(record! { + "name" => Value::test_string("nu"), + "stars" => Value::test_int(5), + "language" => Value::test_string("Rust"), + })), + }, + Example { + description: "Update each row of a table", + example: "[[name lang]; [Nushell ''] [Reedline '']] | upsert lang 'Rust'", + result: Some(Value::test_list(vec![ Value::test_record(record! { "name" => Value::test_string("Nushell"), "lang" => Value::test_string("Rust"), @@ -78,46 +87,45 @@ impl Command for Upsert { "name" => Value::test_string("Reedline"), "lang" => Value::test_string("Rust"), }), - ], - )), - }, - Example { - description: "Insert a new entry into a single record", - example: "{'name': 'nu', 'stars': 5} | upsert language 'Rust'", - result: Some(Value::test_record(record! { - "name" => Value::test_string("nu"), - "stars" => Value::test_int(5), - "language" => Value::test_string("Rust"), - })), - }, Example { - description: "Use in closure form for more involved updating logic", - example: "[[count fruit]; [1 'apple']] | enumerate | upsert item.count {|e| ($e.item.fruit | str length) + $e.index } | get item", - result: Some(Value::test_list( - vec![Value::test_record(record! { - "count" => Value::test_int(5), - "fruit" => Value::test_string("apple"), - })], - )), - }, - Example { - description: "Upsert an int into a list, updating an existing value based on the index", - example: "[1 2 3] | upsert 0 2", - result: Some(Value::test_list( - vec![Value::test_int(2), Value::test_int(2), Value::test_int(3)], - )), - }, - Example { - description: "Upsert an int into a list, inserting a new value based on the index", - example: "[1 2 3] | upsert 3 4", - result: Some(Value::test_list( - vec![ + ])), + }, + Example { + description: "Insert a new column with values computed based off the other columns", + example: "[[foo]; [7] [8] [9]] | upsert bar {|row| $row.foo * 2 }", + result: Some(Value::test_list(vec![ + Value::test_record(record! { + "foo" => Value::test_int(7), + "bar" => Value::test_int(14), + }), + Value::test_record(record! { + "foo" => Value::test_int(8), + "bar" => Value::test_int(16), + }), + Value::test_record(record! { + "foo" => Value::test_int(9), + "bar" => Value::test_int(18), + }), + ])), + }, + Example { + description: "Upsert into a list, updating an existing value at an index", + example: "[1 2 3] | upsert 0 2", + result: Some(Value::test_list(vec![ + Value::test_int(2), + Value::test_int(2), + Value::test_int(3), + ])), + }, + Example { + description: "Upsert into a list, inserting a new value at the end", + example: "[1 2 3] | upsert 3 4", + result: Some(Value::test_list(vec![ Value::test_int(1), Value::test_int(2), Value::test_int(3), Value::test_int(4), - ], - )), - }, + ])), + }, ] } } @@ -128,7 +136,6 @@ fn upsert( call: &Call, input: PipelineData, ) -> Result { - let metadata = input.metadata(); let span = call.head; let cell_path: CellPath = call.req(engine_state, stack, 0)?; @@ -137,101 +144,256 @@ fn upsert( let redirect_stdout = call.redirect_stdout; let redirect_stderr = call.redirect_stderr; - let engine_state = engine_state.clone(); let ctrlc = engine_state.ctrlc.clone(); - // Replace is a block, so set it up and run it instead of using it as the replacement - if replacement.as_block().is_ok() { - let capture_block = Closure::from_value(replacement)?; - let block = engine_state.get_block(capture_block.block_id).clone(); - - let mut stack = stack.captures_to_stack(capture_block.captures); - let orig_env_vars = stack.env_vars.clone(); - let orig_env_hidden = stack.env_hidden.clone(); - - input - .map( - move |mut input| { - // with_env() is used here to ensure that each iteration uses - // a different set of environment variables. - // Hence, a 'cd' in the first loop won't affect the next loop. - stack.with_env(&orig_env_vars, &orig_env_hidden); - - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, input.clone()) + match input { + PipelineData::Value(mut value, metadata) => { + if replacement.as_block().is_ok() { + match (cell_path.members.first(), &mut value) { + (Some(PathMember::String { .. }), Value::List { vals, .. }) => { + let span = replacement.span(); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id); + let stack = stack.captures_to_stack(capture_block.captures.clone()); + for val in vals { + let mut stack = stack.clone(); + upsert_value_by_closure( + val, + span, + engine_state, + &mut stack, + redirect_stdout, + redirect_stderr, + block, + &cell_path.members, + false, + )?; } } - - let output = eval_block( - &engine_state, - &mut stack, - &block, - input.clone().into_pipeline_data(), - redirect_stdout, - redirect_stderr, - ); - - match output { - Ok(pd) => { - if let Err(e) = input - .upsert_data_at_cell_path(&cell_path.members, pd.into_value(span)) - { - return Value::error(e, span); - } - - input - } - Err(e) => Value::error(e, span), + (first, _) => { + upsert_single_value_by_closure( + &mut value, + replacement, + engine_state, + stack, + redirect_stdout, + redirect_stderr, + &cell_path.members, + matches!(first, Some(PathMember::Int { .. })), + )?; } + } + } else { + value.upsert_data_at_cell_path(&cell_path.members, replacement)?; + } + Ok(value.into_pipeline_data_with_metadata(metadata)) + } + PipelineData::ListStream(mut stream, metadata) => { + if let Some(( + &PathMember::Int { + val, + span: path_span, + .. }, - ctrlc, - ) - .map(|x| x.set_metadata(metadata)) - } else { - if let Some(PathMember::Int { val, span, .. }) = cell_path.members.first() { - let mut input = input.into_iter(); - let mut pre_elems = vec![]; + path, + )) = cell_path.members.split_first() + { + let mut pre_elems = vec![]; - for idx in 0..*val { - if let Some(v) = input.next() { - pre_elems.push(v); + for idx in 0..val { + if let Some(v) = stream.next() { + pre_elems.push(v); + } else { + return Err(ShellError::InsertAfterNextFreeIndex { + available_idx: idx, + span: path_span, + }); + } + } + + if path.is_empty() { + let span = replacement.span(); + let value = stream.next().unwrap_or(Value::nothing(span)); + if replacement.as_block().is_ok() { + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id); + let mut stack = stack.captures_to_stack(capture_block.captures); + + if let Some(var) = block.signature.get_positional(0) { + if let Some(var_id) = &var.var_id { + stack.add_var(*var_id, value.clone()) + } + } + + let output = eval_block( + engine_state, + &mut stack, + block, + value.clone().into_pipeline_data(), + redirect_stdout, + redirect_stderr, + )?; + + pre_elems.push(output.into_value(span)); + } else { + pre_elems.push(replacement); + } + } else if let Some(mut value) = stream.next() { + if replacement.as_block().is_ok() { + upsert_single_value_by_closure( + &mut value, + replacement, + engine_state, + stack, + redirect_stdout, + redirect_stderr, + path, + true, + )?; + } else { + value.upsert_data_at_cell_path(path, replacement)?; + } + pre_elems.push(value) } else { return Err(ShellError::AccessBeyondEnd { - max_idx: idx, - span: *span, + max_idx: pre_elems.len() - 1, + span: path_span, }); } + + Ok(pre_elems + .into_iter() + .chain(stream) + .into_pipeline_data_with_metadata(metadata, ctrlc)) + } else if replacement.as_block().is_ok() { + let engine_state = engine_state.clone(); + let replacement_span = replacement.span(); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id).clone(); + let stack = stack.captures_to_stack(capture_block.captures.clone()); + + Ok(stream + .map(move |mut input| { + // Recreate the stack for each iteration to + // isolate environment variable changes, etc. + let mut stack = stack.clone(); + + let err = upsert_value_by_closure( + &mut input, + replacement_span, + &engine_state, + &mut stack, + redirect_stdout, + redirect_stderr, + &block, + &cell_path.members, + false, + ); + + if let Err(e) = err { + Value::error(e, span) + } else { + input + } + }) + .into_pipeline_data_with_metadata(metadata, ctrlc)) + } else { + Ok(stream + .map(move |mut input| { + if let Err(e) = + input.upsert_data_at_cell_path(&cell_path.members, replacement.clone()) + { + Value::error(e, span) + } else { + input + } + }) + .into_pipeline_data_with_metadata(metadata, ctrlc)) } - - // Skip over the replaced value - let _ = input.next(); - - return Ok(pre_elems - .into_iter() - .chain(vec![replacement]) - .chain(input) - .into_pipeline_data_with_metadata(metadata, ctrlc)); } - - input - .map( - move |mut input| { - let replacement = replacement.clone(); - - if let Err(e) = input.upsert_data_at_cell_path(&cell_path.members, replacement) - { - return Value::error(e, span); - } - - input - }, - ctrlc, - ) - .map(|x| x.set_metadata(metadata)) + PipelineData::Empty => Err(ShellError::IncompatiblePathAccess { + type_name: "empty pipeline".to_string(), + span, + }), + PipelineData::ExternalStream { .. } => Err(ShellError::IncompatiblePathAccess { + type_name: "external stream".to_string(), + span, + }), } } +#[allow(clippy::too_many_arguments)] +fn upsert_value_by_closure( + value: &mut Value, + span: Span, + engine_state: &EngineState, + stack: &mut Stack, + redirect_stdout: bool, + redirect_stderr: bool, + block: &Block, + cell_path: &[PathMember], + first_path_member_int: bool, +) -> Result<(), ShellError> { + let input_at_path = value.clone().follow_cell_path(cell_path, false); + + if let Some(var) = block.signature.get_positional(0) { + if let Some(var_id) = &var.var_id { + stack.add_var( + *var_id, + if first_path_member_int { + input_at_path.clone().unwrap_or(Value::nothing(span)) + } else { + value.clone() + }, + ) + } + } + + let input_at_path = input_at_path + .map(IntoPipelineData::into_pipeline_data) + .unwrap_or(PipelineData::Empty); + + let output = eval_block( + engine_state, + stack, + block, + input_at_path, + redirect_stdout, + redirect_stderr, + )?; + + value.upsert_data_at_cell_path(cell_path, output.into_value(span)) +} + +#[allow(clippy::too_many_arguments)] +fn upsert_single_value_by_closure( + value: &mut Value, + replacement: Value, + engine_state: &EngineState, + stack: &mut Stack, + redirect_stdout: bool, + redirect_stderr: bool, + cell_path: &[PathMember], + first_path_member_int: bool, +) -> Result<(), ShellError> { + let span = replacement.span(); + let capture_block = Closure::from_value(replacement)?; + let block = engine_state.get_block(capture_block.block_id); + let mut stack = stack.captures_to_stack(capture_block.captures); + + upsert_value_by_closure( + value, + span, + engine_state, + &mut stack, + redirect_stdout, + redirect_stderr, + block, + cell_path, + first_path_member_int, + ) +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/nu-command/tests/commands/insert.rs b/crates/nu-command/tests/commands/insert.rs index 2044f944a7..f6f2e0bc0e 100644 --- a/crates/nu-command/tests/commands/insert.rs +++ b/crates/nu-command/tests/commands/insert.rs @@ -44,24 +44,49 @@ fn insert_into_list() { } #[test] -fn insert_into_list_begin() { +fn insert_at_start_of_list() { let actual = nu!("[1, 2, 3] | insert 0 abc | to json -r"); assert_eq!(actual.out, r#"["abc",1,2,3]"#); } #[test] -fn insert_into_list_end() { +fn insert_at_end_of_list() { let actual = nu!("[1, 2, 3] | insert 3 abc | to json -r"); assert_eq!(actual.out, r#"[1,2,3,"abc"]"#); } #[test] -fn insert_past_end_list() { - let actual = nu!("[1, 2, 3] | insert 5 abc | to json -r"); +fn insert_past_end_of_list() { + let actual = nu!("[1, 2, 3] | insert 5 abc"); - assert_eq!(actual.out, r#"[1,2,3,null,null,"abc"]"#); + assert!(actual + .err + .contains("can't insert at index (the next available index is 3)")); +} + +#[test] +fn insert_into_list_stream() { + let actual = nu!("[1, 2, 3] | every 1 | insert 1 abc | to json -r"); + + assert_eq!(actual.out, r#"[1,"abc",2,3]"#); +} + +#[test] +fn insert_at_end_of_list_stream() { + let actual = nu!("[1, 2, 3] | every 1 | insert 3 abc | to json -r"); + + assert_eq!(actual.out, r#"[1,2,3,"abc"]"#); +} + +#[test] +fn insert_past_end_of_list_stream() { + let actual = nu!("[1, 2, 3] | every 1 | insert 5 abc"); + + assert!(actual + .err + .contains("can't insert at index (the next available index is 3)")); } #[test] @@ -90,14 +115,82 @@ fn lazy_record_test_values() { #[test] fn deep_cell_path_creates_all_nested_records() { - let actual = nu!(r#"{a: {}} | insert a.b.c 0 | get a.b.c"#); + let actual = nu!("{a: {}} | insert a.b.c 0 | get a.b.c"); assert_eq!(actual.out, "0"); } #[test] fn inserts_all_rows_in_table_in_record() { let actual = nu!( - r#"{table: [[col]; [{a: 1}], [{a: 1}]]} | insert table.col.b 2 | get table.col.b | to nuon"# + "{table: [[col]; [{a: 1}], [{a: 1}]]} | insert table.col.b 2 | get table.col.b | to nuon" ); assert_eq!(actual.out, "[2, 2]"); } + +#[test] +fn list_replacement_closure() { + let actual = nu!("[1, 2] | insert 1 {|i| $i + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3, 2]"); + + let actual = nu!("[1, 2] | insert 1 { $in + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3, 2]"); + + let actual = nu!("[1, 2] | insert 2 {|i| if $i == null { 0 } else { $in + 1 } } | to nuon"); + assert_eq!(actual.out, "[1, 2, 0]"); + + let actual = nu!("[1, 2] | insert 2 { if $in == null { 0 } else { $in + 1 } } | to nuon"); + assert_eq!(actual.out, "[1, 2, 0]"); +} + +#[test] +fn record_replacement_closure() { + let actual = nu!("{ a: text } | insert b {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "{a: text, b: TEXT}"); + + let actual = nu!("{ a: text } | insert b { default TEXT } | to nuon"); + assert_eq!(actual.out, "{a: text, b: TEXT}"); + + let actual = nu!("{ a: { b: 1 } } | insert a.c {|r| $r.a.b } | to nuon"); + assert_eq!(actual.out, "{a: {b: 1, c: 1}}"); + + let actual = nu!("{ a: { b: 1 } } | insert a.c { default 0 } | to nuon"); + assert_eq!(actual.out, "{a: {b: 1, c: 0}}"); +} + +#[test] +fn table_replacement_closure() { + let actual = nu!("[[a]; [text]] | insert b {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "[[a, b]; [text, TEXT]]"); + + let actual = nu!("[[a]; [text]] | insert b { default TEXT } | to nuon"); + assert_eq!(actual.out, "[[a, b]; [text, TEXT]]"); + + let actual = nu!("[[b]; [1]] | wrap a | insert a.c {|r| $r.a.b } | to nuon"); + assert_eq!(actual.out, "[[a]; [{b: 1, c: 1}]]"); + + let actual = nu!("[[b]; [1]] | wrap a | insert a.c { default 0 } | to nuon"); + assert_eq!(actual.out, "[[a]; [{b: 1, c: 0}]]"); +} + +#[test] +fn list_stream_replacement_closure() { + let actual = nu!("[1, 2] | every 1 | insert 1 {|i| $i + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3, 2]"); + + let actual = nu!("[1, 2] | every 1 | insert 1 { $in + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3, 2]"); + + let actual = + nu!("[1, 2] | every 1 | insert 2 {|i| if $i == null { 0 } else { $in + 1 } } | to nuon"); + assert_eq!(actual.out, "[1, 2, 0]"); + + let actual = + nu!("[1, 2] | every 1 | insert 2 { if $in == null { 0 } else { $in + 1 } } | to nuon"); + assert_eq!(actual.out, "[1, 2, 0]"); + + let actual = nu!("[[a]; [text]] | every 1 | insert b {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "[[a, b]; [text, TEXT]]"); + + let actual = nu!("[[a]; [text]] | every 1 | insert b { default TEXT } | to nuon"); + assert_eq!(actual.out, "[[a, b]; [text, TEXT]]"); +} diff --git a/crates/nu-command/tests/commands/update.rs b/crates/nu-command/tests/commands/update.rs index 897f878e0b..2e74657f7c 100644 --- a/crates/nu-command/tests/commands/update.rs +++ b/crates/nu-command/tests/commands/update.rs @@ -71,11 +71,23 @@ fn update_list() { } #[test] -fn update_past_end_list() { +fn update_past_end_of_list() { let actual = nu!("[1, 2, 3] | update 5 abc | to json -r"); assert!(actual.err.contains("too large")); } +#[test] +fn update_list_stream() { + let actual = nu!("[1, 2, 3] | every 1 | update 1 abc | to json -r"); + assert_eq!(actual.out, r#"[1,"abc",3]"#); +} + +#[test] +fn update_past_end_of_list_stream() { + let actual = nu!("[1, 2, 3] | every 1 | update 5 abc | to json -r"); + assert!(actual.err.contains("too large")); +} + #[test] fn update_nonexistent_column() { let actual = nu!("{a:1} | update b 2"); @@ -85,7 +97,7 @@ fn update_nonexistent_column() { #[test] fn update_uses_enumerate_index() { let actual = nu!( - r#"[[a]; [7] [6]] | enumerate | update item.a {|el| $el.index + 1 + $el.item.a } | flatten | to nuon"# + "[[a]; [7] [6]] | enumerate | update item.a {|el| $el.index + 1 + $el.item.a } | flatten | to nuon" ); assert_eq!(actual.out, "[[index, a]; [0, 8], [1, 8]]"); @@ -97,3 +109,45 @@ fn update_support_lazy_record() { nu!(r#"let x = (lazy make -c ["h"] -g {|a| $a | str upcase}); $x | update h 10 | get h"#); assert_eq!(actual.out, "10"); } + +#[test] +fn list_replacement_closure() { + let actual = nu!("[1, 2] | update 1 {|i| $i + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3]"); + + let actual = nu!("[1, 2] | update 1 { $in + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3]"); +} + +#[test] +fn record_replacement_closure() { + let actual = nu!("{ a: text } | update a {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "{a: TEXT}"); + + let actual = nu!("{ a: text } | update a { str upcase } | to nuon"); + assert_eq!(actual.out, "{a: TEXT}"); +} + +#[test] +fn table_replacement_closure() { + let actual = nu!("[[a]; [text]] | update a {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "[[a]; [TEXT]]"); + + let actual = nu!("[[a]; [text]] | update a { str upcase } | to nuon"); + assert_eq!(actual.out, "[[a]; [TEXT]]"); +} + +#[test] +fn list_stream_replacement_closure() { + let actual = nu!("[1, 2] | every 1 | update 1 {|i| $i + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3]"); + + let actual = nu!("[1, 2] | every 1 | update 1 { $in + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3]"); + + let actual = nu!("[[a]; [text]] | every 1 | update a {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "[[a]; [TEXT]]"); + + let actual = nu!("[[a]; [text]] | every 1 | update a { str upcase } | to nuon"); + assert_eq!(actual.out, "[[a]; [TEXT]]"); +} diff --git a/crates/nu-command/tests/commands/upsert.rs b/crates/nu-command/tests/commands/upsert.rs index 14395ea2be..1bb823e88c 100644 --- a/crates/nu-command/tests/commands/upsert.rs +++ b/crates/nu-command/tests/commands/upsert.rs @@ -67,17 +67,49 @@ fn upsert_uses_enumerate_index_updating() { } #[test] -fn index_does_not_exist() { - let actual = nu!("[1,2,3] | upsert 4 4"); +fn upsert_into_list() { + let actual = nu!("[1, 2, 3] | upsert 1 abc | to json -r"); - assert!(actual.err.contains("index too large (max: 3)")); + assert_eq!(actual.out, r#"[1,"abc",3]"#); } #[test] -fn upsert_empty() { - let actual = nu!("[] | upsert 1 1"); +fn upsert_at_end_of_list() { + let actual = nu!("[1, 2, 3] | upsert 3 abc | to json -r"); - assert!(actual.err.contains("index too large (max: 0)")); + assert_eq!(actual.out, r#"[1,2,3,"abc"]"#); +} + +#[test] +fn upsert_past_end_of_list() { + let actual = nu!("[1, 2, 3] | upsert 5 abc"); + + assert!(actual + .err + .contains("can't insert at index (the next available index is 3)")); +} + +#[test] +fn upsert_into_list_stream() { + let actual = nu!("[1, 2, 3] | every 1 | upsert 1 abc | to json -r"); + + assert_eq!(actual.out, r#"[1,"abc",3]"#); +} + +#[test] +fn upsert_at_end_of_list_stream() { + let actual = nu!("[1, 2, 3] | every 1 | upsert 3 abc | to json -r"); + + assert_eq!(actual.out, r#"[1,2,3,"abc"]"#); +} + +#[test] +fn upsert_past_end_of_list_stream() { + let actual = nu!("[1, 2, 3] | every 1 | upsert 5 abc"); + + assert!(actual + .err + .contains("can't insert at index (the next available index is 3)")); } #[test] @@ -93,14 +125,100 @@ fn upsert_support_lazy_record() { #[test] fn deep_cell_path_creates_all_nested_records() { - let actual = nu!(r#"{a: {}} | insert a.b.c 0 | get a.b.c"#); + let actual = nu!("{a: {}} | upsert a.b.c 0 | get a.b.c"); assert_eq!(actual.out, "0"); } #[test] fn upserts_all_rows_in_table_in_record() { let actual = nu!( - r#"{table: [[col]; [{a: 1}], [{a: 1}]]} | insert table.col.b 2 | get table.col.b | to nuon"# + "{table: [[col]; [{a: 1}], [{a: 1}]]} | upsert table.col.b 2 | get table.col.b | to nuon" ); assert_eq!(actual.out, "[2, 2]"); } + +#[test] +fn list_replacement_closure() { + let actual = nu!("[1, 2] | upsert 1 {|i| $i + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3]"); + + let actual = nu!("[1, 2] | upsert 1 { $in + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3]"); + + let actual = nu!("[1, 2] | upsert 2 {|i| if $i == null { 0 } else { $in + 1 } } | to nuon"); + assert_eq!(actual.out, "[1, 2, 0]"); + + let actual = nu!("[1, 2] | upsert 2 { if $in == null { 0 } else { $in + 1 } } | to nuon"); + assert_eq!(actual.out, "[1, 2, 0]"); +} + +#[test] +fn record_replacement_closure() { + let actual = nu!("{ a: text } | upsert a {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "{a: TEXT}"); + + let actual = nu!("{ a: text } | upsert a { str upcase } | to nuon"); + assert_eq!(actual.out, "{a: TEXT}"); + + let actual = nu!("{ a: text } | upsert b {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "{a: text, b: TEXT}"); + + let actual = nu!("{ a: text } | upsert b { default TEXT } | to nuon"); + assert_eq!(actual.out, "{a: text, b: TEXT}"); + + let actual = nu!("{ a: { b: 1 } } | upsert a.c {|r| $r.a.b } | to nuon"); + assert_eq!(actual.out, "{a: {b: 1, c: 1}}"); + + let actual = nu!("{ a: { b: 1 } } | upsert a.c { default 0 } | to nuon"); + assert_eq!(actual.out, "{a: {b: 1, c: 0}}"); +} + +#[test] +fn table_replacement_closure() { + let actual = nu!("[[a]; [text]] | upsert a {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "[[a]; [TEXT]]"); + + let actual = nu!("[[a]; [text]] | upsert a { str upcase } | to nuon"); + assert_eq!(actual.out, "[[a]; [TEXT]]"); + + let actual = nu!("[[a]; [text]] | upsert b {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "[[a, b]; [text, TEXT]]"); + + let actual = nu!("[[a]; [text]] | upsert b { default TEXT } | to nuon"); + assert_eq!(actual.out, "[[a, b]; [text, TEXT]]"); + + let actual = nu!("[[b]; [1]] | wrap a | upsert a.c {|r| $r.a.b } | to nuon"); + assert_eq!(actual.out, "[[a]; [{b: 1, c: 1}]]"); + + let actual = nu!("[[b]; [1]] | wrap a | upsert a.c { default 0 } | to nuon"); + assert_eq!(actual.out, "[[a]; [{b: 1, c: 0}]]"); +} + +#[test] +fn list_stream_replacement_closure() { + let actual = nu!("[1, 2] | every 1 | upsert 1 {|i| $i + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3]"); + + let actual = nu!("[1, 2] | every 1 | upsert 1 { $in + 1 } | to nuon"); + assert_eq!(actual.out, "[1, 3]"); + + let actual = + nu!("[1, 2] | every 1 | upsert 2 {|i| if $i == null { 0 } else { $in + 1 } } | to nuon"); + assert_eq!(actual.out, "[1, 2, 0]"); + + let actual = + nu!("[1, 2] | every 1 | upsert 2 { if $in == null { 0 } else { $in + 1 } } | to nuon"); + assert_eq!(actual.out, "[1, 2, 0]"); + + let actual = nu!("[[a]; [text]] | every 1 | upsert a {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "[[a]; [TEXT]]"); + + let actual = nu!("[[a]; [text]] | every 1 | upsert a { str upcase } | to nuon"); + assert_eq!(actual.out, "[[a]; [TEXT]]"); + + let actual = nu!("[[a]; [text]] | every 1 | upsert b {|r| $r.a | str upcase } | to nuon"); + assert_eq!(actual.out, "[[a, b]; [text, TEXT]]"); + + let actual = nu!("[[a]; [text]] | every 1 | upsert b { default TEXT } | to nuon"); + assert_eq!(actual.out, "[[a, b]; [text, TEXT]]"); +} diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 4a486fc4c1..4feea256a0 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -1143,6 +1143,7 @@ impl Value { cell_path: &[PathMember], new_val: Value, ) -> Result<(), ShellError> { + let v_span = self.span(); if let Some((member, path)) = cell_path.split_first() { match member { PathMember::String { @@ -1215,22 +1216,26 @@ impl Value { Value::List { vals, .. } => { if let Some(v) = vals.get_mut(*row_num) { v.upsert_data_at_cell_path(path, new_val)?; - } else if vals.len() == *row_num && path.is_empty() { - // If the upsert is at 1 + the end of the list, it's OK. - // Otherwise, it's prohibited. - vals.push(new_val); - } else { + } else if vals.len() != *row_num { return Err(ShellError::InsertAfterNextFreeIndex { available_idx: vals.len(), span: *span, }); + } else if !path.is_empty() { + return Err(ShellError::AccessBeyondEnd { + max_idx: vals.len() - 1, + span: *span, + }); + } else { + // If the upsert is at 1 + the end of the list, it's OK. + vals.push(new_val); } } Value::Error { error, .. } => return Err(*error.clone()), - v => { + _ => { return Err(ShellError::NotAList { dst_span: *span, - src_span: v.span(), + src_span: v_span, }); } }, @@ -1632,22 +1637,30 @@ impl Value { } => match self { Value::List { vals, .. } => { if let Some(v) = vals.get_mut(*row_num) { - v.insert_data_at_cell_path(path, new_val, head_span)?; - } else if vals.len() == *row_num && path.is_empty() { - // If the insert is at 1 + the end of the list, it's OK. - // Otherwise, it's prohibited. - vals.push(new_val); - } else { + if path.is_empty() { + vals.insert(*row_num, new_val); + } else { + v.insert_data_at_cell_path(path, new_val, head_span)?; + } + } else if vals.len() != *row_num { return Err(ShellError::InsertAfterNextFreeIndex { available_idx: vals.len(), span: *span, }); + } else if !path.is_empty() { + return Err(ShellError::AccessBeyondEnd { + max_idx: vals.len() - 1, + span: *span, + }); + } else { + // If the insert is at 1 + the end of the list, it's OK. + vals.push(new_val); } } - v => { + _ => { return Err(ShellError::NotAList { dst_span: *span, - src_span: v.span(), + src_span: v_span, }); } },