From c95026957578e3029e0c2ccd333a3c5c7cebecb0 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Thu, 14 Mar 2024 21:43:03 +0000 Subject: [PATCH] Fix `$in` value for `insert` closure (#12209) # Description Fixes #12193 where the `$in` value may be null for closures provided to `insert`. # User-Facing Changes The `$in` value will now always be the same as the closure parameter for `insert`. --- crates/nu-command/src/filters/insert.rs | 37 +++++++++++----------- crates/nu-command/src/filters/update.rs | 9 +++++- crates/nu-command/src/filters/upsert.rs | 8 +++++ crates/nu-command/tests/commands/insert.rs | 14 ++++---- crates/nu-protocol/src/value/mod.rs | 8 ++--- 5 files changed, 44 insertions(+), 32 deletions(-) diff --git a/crates/nu-command/src/filters/insert.rs b/crates/nu-command/src/filters/insert.rs index f738ca87c3..8feac01d9b 100644 --- a/crates/nu-command/src/filters/insert.rs +++ b/crates/nu-command/src/filters/insert.rs @@ -43,6 +43,11 @@ impl Command for Insert { "Insert a new column, using an expression or closure to create each row's values." } + fn extra_usage(&self) -> &str { + "When inserting a column, the closure will be run for each row, and the current row will be passed as the first argument. +When inserting into a specific index, the closure will instead get the current value at the index or null if inserting at the end of a list/table." + } + fn search_terms(&self) -> Vec<&str> { vec!["add"] } @@ -63,7 +68,7 @@ impl Command for Insert { 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"), + "name" => Value::test_string("nu"), "stars" => Value::test_int(5), "alias" => Value::test_string("Nushell"), })), @@ -73,8 +78,8 @@ impl Command for Insert { 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"), + "lang" => Value::test_string("Rust"), + "type" => Value::test_string("shell"), })])), }, Example { @@ -322,26 +327,22 @@ fn insert_value_by_closure( first_path_member_int: bool, eval_block_fn: EvalBlockFn, ) -> Result<(), ShellError> { - let input_at_path = value.clone().follow_cell_path(cell_path, false); + let input = if first_path_member_int { + value + .clone() + .follow_cell_path(cell_path, false) + .unwrap_or(Value::nothing(span)) + } else { + value.clone() + }; 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() - }, - ) + if let Some(var_id) = var.var_id { + stack.add_var(var_id, input.clone()); } } - let input_at_path = input_at_path - .map(IntoPipelineData::into_pipeline_data) - .unwrap_or(PipelineData::Empty); - - let output = eval_block_fn(engine_state, stack, block, input_at_path)?; + let output = eval_block_fn(engine_state, stack, block, input.into_pipeline_data())?; value.insert_data_at_cell_path(cell_path, output.into_value(span), span) } diff --git a/crates/nu-command/src/filters/update.rs b/crates/nu-command/src/filters/update.rs index 6b7c482974..d8f9847bf3 100644 --- a/crates/nu-command/src/filters/update.rs +++ b/crates/nu-command/src/filters/update.rs @@ -43,6 +43,13 @@ impl Command for Update { "Update an existing column to have a new value." } + fn extra_usage(&self) -> &str { + "When updating a column, the closure will be run for each row, and the current row will be passed as the first argument. \ +Referencing `$in` inside the closure will provide the value at the column for the current row. + +When updating a specific index, the closure will instead be run once. The first argument to the closure and the `$in` value will both be the current value at the index." + } + fn run( &self, engine_state: &EngineState, @@ -74,7 +81,7 @@ impl Command for Update { )), }, Example { - description: "You can also use a simple command to update 'authors' to a single string", + description: "Implicitly use the `$in` value in a closure to update 'authors'", example: "[[project, authors]; ['nu', ['Andrés', 'JT', 'Yehuda']]] | update authors { str join ',' }", result: Some(Value::test_list( vec![Value::test_record(record! { diff --git a/crates/nu-command/src/filters/upsert.rs b/crates/nu-command/src/filters/upsert.rs index 6b1e81a058..05d81ea3b4 100644 --- a/crates/nu-command/src/filters/upsert.rs +++ b/crates/nu-command/src/filters/upsert.rs @@ -43,6 +43,14 @@ impl Command for Upsert { "Update an existing column to have a new value, or insert a new column." } + fn extra_usage(&self) -> &str { + "When updating or inserting a column, the closure will be run for each row, and the current row will be passed as the first argument. \ +Referencing `$in` inside the closure will provide the value at the column for the current row or null if the column does not exist. + +When updating a specific index, the closure will instead be run once. The first argument to the closure and the `$in` value will both be the current value at the index. \ +If the command is inserting at the end of a list or table, then both of these values will be null." + } + fn search_terms(&self) -> Vec<&str> { vec!["add"] } diff --git a/crates/nu-command/tests/commands/insert.rs b/crates/nu-command/tests/commands/insert.rs index f6f2e0bc0e..400319c4f0 100644 --- a/crates/nu-command/tests/commands/insert.rs +++ b/crates/nu-command/tests/commands/insert.rs @@ -147,14 +147,14 @@ 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"); + let actual = nu!("{ a: text } | insert b { $in.a | str upcase } | 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}}"); + let actual = nu!("{ a: { b: 1 } } | insert a.c { $in.a.b } | to nuon"); + assert_eq!(actual.out, "{a: {b: 1, c: 1}}"); } #[test] @@ -162,14 +162,14 @@ 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"); + let actual = nu!("[[a]; [text]] | insert b { $in.a | str upcase } | 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}]]"); + let actual = nu!("[[b]; [1]] | wrap a | insert a.c { $in.a.b } | to nuon"); + assert_eq!(actual.out, "[[a]; [{b: 1, c: 1}]]"); } #[test] @@ -191,6 +191,6 @@ fn list_stream_replacement_closure() { 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"); + let actual = nu!("[[a]; [text]] | every 1 | insert b { $in.a | str upcase } | 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 82b83f172f..8b6f32693f 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -1766,14 +1766,10 @@ impl Value { } } else { let new_col = if path.is_empty() { - new_val.clone() + new_val } else { let mut new_col = Value::record(Record::new(), new_val.span()); - new_col.insert_data_at_cell_path( - path, - new_val.clone(), - head_span, - )?; + new_col.insert_data_at_cell_path(path, new_val, head_span)?; new_col }; record.push(col_name, new_col);