Skip to content

Commit

Permalink
Modify reject algorithm for identical elements (#8446)
Browse files Browse the repository at this point in the history
# 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
    }
});
```
  • Loading branch information
tcoratger committed Mar 14, 2023
1 parent 1701303 commit 0bd4d27
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
29 changes: 29 additions & 0 deletions crates/nu-command/tests/commands/reject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
12 changes: 8 additions & 4 deletions crates/nu-protocol/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down

0 comments on commit 0bd4d27

Please sign in to comment.