From 651e86a3c08fdbac69a976eace0bc3ee9767bd99 Mon Sep 17 00:00:00 2001 From: raccmonteiro <3062698+raccmonteiro@users.noreply.github.com> Date: Wed, 23 Nov 2022 23:46:20 +0000 Subject: [PATCH] `uniq -i` does not convert to lowercase (#7192) (#7209) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description `uniq -i` does not convert output strings to lowercase. Also, `uniq -i` did not ignore case in strings below the first level of Tables and Records. Now all strings case are ignored for all children Values for tables, Records, and List. Fixes https://github.com/nushell/nushell/issues/7192 # Tests + Formatting About the issue https://github.com/nushell/nushell/issues/7192, the output will be: ``` 〉[AAA BBB CCC] | uniq -i ╭───┬─────╮ │ 0 │ AAA │ │ 1 │ BBB │ │ 2 │ CCC │ ╰───┴─────╯ ``` About ignoring case for all children string, I expect this to be true: ``` ([[origin, people]; [World, ( [[name, meal]; ['Geremias', {plate: 'bitoque', carbs: 100}] ] )], [World, ( [[name, meal]; ['Martin', {plate: 'bitoque', carbs: 100}] ] )], [World, ( [[name, meal]; ['Geremias', {plate: 'Bitoque', carbs: 100}] ] )], ] | uniq -i ) == ([[origin, people]; [World, ( [[name, meal]; ['Geremias', {plate: 'bitoque', carbs: 100}] ] )], [World, ( [[name, meal]; ['Martin', {plate: 'bitoque', carbs: 100}] ] )] ]) ``` --- crates/nu-command/src/filters/uniq.rs | 88 +++++++++++++++++------- crates/nu-command/tests/commands/uniq.rs | 55 ++++++++++++++- 2 files changed, 116 insertions(+), 27 deletions(-) diff --git a/crates/nu-command/src/filters/uniq.rs b/crates/nu-command/src/filters/uniq.rs index dfd198f6a7..68fe5b4a7d 100644 --- a/crates/nu-command/src/filters/uniq.rs +++ b/crates/nu-command/src/filters/uniq.rs @@ -123,25 +123,68 @@ impl Command for Uniq { } } -fn to_lowercase(value: nu_protocol::Value) -> nu_protocol::Value { - match value { - Value::String { val: s, span } => Value::String { - val: s.to_lowercase(), - span, - }, - other => other, +struct ValueCounter { + val: Value, + val_to_compare: Value, + count: i64, +} + +impl PartialEq for ValueCounter { + fn eq(&self, other: &Self) -> bool { + self.val == other.val } } -fn generate_results_with_count(head: Span, uniq_values: Vec<(Value, i64)>) -> Vec { +impl ValueCounter { + fn new(val: Value, flag_ignore_case: bool) -> Self { + ValueCounter { + val: val.clone(), + val_to_compare: if flag_ignore_case { + clone_to_lowercase(&val) + } else { + val + }, + count: 1, + } + } +} + +fn clone_to_lowercase(value: &Value) -> Value { + match value { + Value::String { val: s, span } => Value::String { + val: s.clone().to_lowercase(), + span: *span, + }, + Value::List { vals: vec, span } => Value::List { + vals: vec + .clone() + .into_iter() + .map(|v| clone_to_lowercase(&v)) + .collect(), + span: *span, + }, + Value::Record { cols, vals, span } => Value::Record { + cols: cols.clone(), + vals: vals + .clone() + .into_iter() + .map(|v| clone_to_lowercase(&v)) + .collect(), + span: *span, + }, + other => other.clone(), + } +} + +fn generate_results_with_count(head: Span, uniq_values: Vec) -> Vec { uniq_values .into_iter() .map(|item| Value::Record { cols: vec!["value".to_string(), "count".to_string()], vals: vec![ - item.0, + item.val, Value::Int { - val: item.1, + val: item.count, span: head, }, ], @@ -165,33 +208,30 @@ fn uniq( let mut uniq_values = input .into_iter() - .map(|item| { - if flag_ignore_case { - to_lowercase(item) - } else { - item - } - }) - .fold(Vec::<(Value, i64)>::new(), |mut counter, item| { - match counter.iter_mut().find(|x| x.0 == item) { - Some(x) => x.1 += 1, - None => counter.push((item, 1)), + .map(|item| ValueCounter::new(item, flag_ignore_case)) + .fold(Vec::::new(), |mut counter, item| { + match counter + .iter_mut() + .find(|x| x.val_to_compare == item.val_to_compare) + { + Some(x) => x.count += 1, + None => counter.push(item), }; counter }); if flag_show_repeated { - uniq_values.retain(|value_count_pair| value_count_pair.1 > 1); + uniq_values.retain(|value_count_pair| value_count_pair.count > 1); } if flag_only_uniques { - uniq_values.retain(|value_count_pair| value_count_pair.1 == 1); + uniq_values.retain(|value_count_pair| value_count_pair.count == 1); } let result = if flag_show_count { generate_results_with_count(head, uniq_values) } else { - uniq_values.into_iter().map(|v| v.0).collect() + uniq_values.into_iter().map(|v| v.val).collect() }; Ok(Value::List { diff --git a/crates/nu-command/tests/commands/uniq.rs b/crates/nu-command/tests/commands/uniq.rs index 26f552a879..70d98b3b6b 100644 --- a/crates/nu-command/tests/commands/uniq.rs +++ b/crates/nu-command/tests/commands/uniq.rs @@ -232,11 +232,11 @@ fn uniq_simple_vals_strs() { } #[test] -fn with_table() { +fn table() { let actual = nu!( cwd: "tests/fixtures/formats", pipeline( r#" - [[fruit day]; [apple monday] [apple friday] [apple monday] [pear monday] [orange tuesday]] + [[fruit day]; [apple monday] [apple friday] [Apple friday] [apple monday] [pear monday] [orange tuesday]] | uniq "# )); @@ -244,10 +244,59 @@ fn with_table() { let expected = nu!( cwd: "tests/fixtures/formats", pipeline( r#" - echo [[fruit day]; [apple monday] [apple friday] [pear monday] [orange tuesday]] + echo [[fruit day]; [apple monday] [apple friday] [Apple friday] [pear monday] [orange tuesday]] "# )); print!("{}", actual.out); print!("{}", expected.out); assert_eq!(actual.out, expected.out); } + +#[test] +fn table_with_ignore_case() { + let actual = nu!( + cwd: "tests/fixtures/formats", pipeline( + r#" + [[origin, people]; + [World, ( + [[name, meal]; + ['Geremias', {plate: 'bitoque', carbs: 100}] + ] + )], + [World, ( + [[name, meal]; + ['Martin', {plate: 'bitoque', carbs: 100}] + ] + )], + [World, ( + [[name, meal]; + ['Geremias', {plate: 'Bitoque', carbs: 100}] + ] + )], + ] | uniq -i + "# + )); + + let expected = nu!( + cwd: "tests/fixtures/formats", pipeline( + r#" + echo [[origin, people]; + [World, ( + [[name, meal]; + ['Geremias', {plate: 'bitoque', carbs: 100}] + ] + )], + [World, ( + [[name, meal]; + ['Martin', {plate: 'bitoque', carbs: 100}] + ] + )], + ] + "# + )); + + print!("{}", actual.out); + print!("{}", expected.out); + assert_eq!(actual.out, expected.out); + assert_eq!(actual.out, expected.out); +}