From 0bd4d27e8d9beedbb97bea73b9411a6d88e2aa7d Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Tue, 14 Mar 2023 23:26:48 +0100 Subject: [PATCH] Modify reject algorithm for identical elements (#8446) # Description The correction made here concerns the issue #8431. Indeed, the algorithm initially proposed to remove elements of a `vector` performed a loop with `remove` and an incident therefore appeared when several values were equal because the deletion was done outside the length of the vector: ```rust let mut found = false; for (i, col) in cols.clone().iter().enumerate() { if col == col_name { cols.remove(i); vals.remove(i); found = true; } } ``` Then, `[[a, a]; [1, 2]] | reject a: ` gave `thread 'main' panicked at 'removal index (is 1) should be < len (is 1)', crates/nu-protocol/src/value/mod.rs:1213:54`. The proposed correction is therefore the implementation of the `retain_mut` utility dedicated to this functionality. ```rust let mut found = false; let mut index = 0; cols.retain_mut(|col| { if col == col_name { found = true; vals.remove(index); false } else { index += 1; true } }); ``` --- crates/nu-command/tests/commands/reject.rs | 29 ++++++++++++++++++++++ crates/nu-protocol/src/value/mod.rs | 12 ++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/crates/nu-command/tests/commands/reject.rs b/crates/nu-command/tests/commands/reject.rs index 7d9d072e5..f9dd15c85 100644 --- a/crates/nu-command/tests/commands/reject.rs +++ b/crates/nu-command/tests/commands/reject.rs @@ -118,3 +118,32 @@ fn reject_nested_field() { assert_eq!(actual.out, "{a: {c: 5}}"); } + +#[test] +fn reject_two_identical_elements() { + let actual = nu!( + cwd: ".", pipeline( + r#"[[a, a]; [1, 2]] | reject a"# + ) + ); + assert!(actual.out.contains("record 0 fields")); +} + +#[test] +fn reject_large_vec_with_two_identical_elements() { + let actual = nu!( + cwd: ".", pipeline( + r#"[[a, b, c, d, e, a]; [1323, 23, 45, 100, 2, 2423]] | reject a"# + ) + ); + assert!(!actual.out.contains("1323")); + assert!(!actual.out.contains("2423")); + assert!(actual.out.contains('b')); + assert!(actual.out.contains('c')); + assert!(actual.out.contains('d')); + assert!(actual.out.contains('e')); + assert!(actual.out.contains("23")); + assert!(actual.out.contains("45")); + assert!(actual.out.contains("100")); + assert!(actual.out.contains('2')); +} diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index c0c00a324..ebc44a2a7 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -1231,13 +1231,17 @@ impl Value { span: v_span, } => { let mut found = false; - for (i, col) in cols.clone().iter().enumerate() { + let mut index = 0; + cols.retain_mut(|col| { if col == col_name { - cols.remove(i); - vals.remove(i); found = true; + vals.remove(index); + false + } else { + index += 1; + true } - } + }); if !found { return Err(ShellError::CantFindColumn { col_name: col_name.to_string(),