From cdc4fb101145cc5a3fc9c95f62ea87b98ae29e1e Mon Sep 17 00:00:00 2001 From: mengsuenyan <49850817+mengsuenyan@users.noreply.github.com> Date: Fri, 21 Jul 2023 21:25:06 +0800 Subject: [PATCH] fix #9653 the cmd `detect columns` with the flag `-c` (#9667) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix `detect columns` with flag `-c, --combine-columns` run failed when using some range - fixes #9653 fix #9653 the cmd detect columns with the flag -c, --combine-columns run failed when using some range. add unit test for the command `detect columns` ```text Attempt to automatically split text into multiple columns. Usage: > detect columns {flags} Flags: -h, --help - Display the help message for this command -s, --skip - number of rows to skip before detecting -n, --no-headers - don't detect headers -c, --combine-columns - columns to be combined; listed as a range Signatures: | detect columns -> Examples: Splits string across multiple columns > 'a b c' | detect columns -n ╭───┬─────────┬─────────┬─────────╮ │ # │ column0 │ column1 │ column2 │ ├───┼─────────┼─────────┼─────────┤ │ 0 │ a │ b │ c │ ╰───┴─────────┴─────────┴─────────╯ Splits a multi-line string into columns with headers detected > $'c1 c2 c3 c4 c5(char nl)a b c d e' | detect columns ╭───┬────┬────┬────┬────┬────╮ │ # │ c1 │ c2 │ c3 │ c4 │ c5 │ ├───┼────┼────┼────┼────┼────┤ │ 0 │ a │ b │ c │ d │ e │ ╰───┴────┴────┴────┴────┴────╯ > $'c1 c2 c3 c4 c5(char nl)a b c d e' | detect columns -c 0..1 ╭───┬─────┬────┬────┬────╮ │ # │ c1 │ c3 │ c4 │ c5 │ ├───┼─────┼────┼────┼────┤ │ 0 │ a b │ c │ d │ e │ ╰───┴─────┴────┴────┴────╯ Splits a multi-line string into columns with headers detected > $'c1 c2 c3 c4 c5(char nl)a b c d e' | detect columns -c -2..-1 ╭───┬────┬────┬────┬─────╮ │ # │ c1 │ c2 │ c3 │ c4 │ ├───┼────┼────┼────┼─────┤ │ 0 │ a │ b │ c │ d e │ ╰───┴────┴────┴────┴─────╯ Splits a multi-line string into columns with headers detected > $'c1 c2 c3 c4 c5(char nl)a b c d e' | detect columns -c 2.. ╭───┬────┬────┬───────╮ │ # │ c1 │ c2 │ c3 │ ├───┼────┼────┼───────┤ │ 0 │ a │ b │ c d e │ ╰───┴────┴────┴───────╯ Parse external ls command and combine columns for datetime > ^ls -lh | detect columns --no-headers --skip 1 --combine-columns 5..7 ``` --- .../nu-command/src/strings/detect_columns.rs | 123 ++++++++++-------- .../tests/commands/detect_columns.rs | 65 +++++++++ crates/nu-command/tests/commands/mod.rs | 1 + 3 files changed, 132 insertions(+), 57 deletions(-) create mode 100644 crates/nu-command/tests/commands/detect_columns.rs diff --git a/crates/nu-command/src/strings/detect_columns.rs b/crates/nu-command/src/strings/detect_columns.rs index cd13f9df70..71262bde5f 100644 --- a/crates/nu-command/src/strings/detect_columns.rs +++ b/crates/nu-command/src/strings/detect_columns.rs @@ -80,9 +80,19 @@ impl Command for DetectColumns { span, }), }, + Example { + description: "", + example: "$'c1 c2 c3 c4 c5(char nl)a b c d e' | detect columns -c 0..1", + result: None, + }, Example { description: "Splits a multi-line string into columns with headers detected", - example: "$'c1 c2 c3(char nl)a b c' | detect columns", + example: "$'c1 c2 c3 c4 c5(char nl)a b c d e' | detect columns -c -2..-1", + result: None, + }, + Example { + description: "Splits a multi-line string into columns with headers detected", + example: "$'c1 c2 c3 c4 c5(char nl)a b c d e' | detect columns -c 2..", result: None, }, Example { @@ -185,70 +195,69 @@ fn detect_columns( } } - if range.is_some() { - // Destructure the range parameter - let (start_index, end_index) = if let Some(range) = &range { - match nu_cmd_base::util::process_range(range) { - Ok(r) => { - // `process_range()` returns `isize::MAX` if the range is open-ended, - // which is not ideal for us - let end = if r.1 as usize > cols.len() { - cols.len() - } else { - r.1 as usize - }; - (r.0 as usize, end) - } - Err(processing_error) => { - let err = processing_error("could not find range index", name_span); - return Value::Error { - error: Box::new(err), + let (start_index, end_index) = if let Some(range) = &range { + match nu_cmd_base::util::process_range(range) { + Ok((l_idx, r_idx)) => { + let l_idx = if l_idx < 0 { + cols.len() as isize + l_idx + } else { + l_idx + }; + + let r_idx = if r_idx < 0 { + cols.len() as isize + r_idx + } else { + r_idx + }; + + if !(l_idx <= r_idx && (r_idx >= 0 || l_idx < (cols.len() as isize))) { + return Value::Record { + cols, + vals, + span: name_span, }; } + + (l_idx.max(0) as usize, (r_idx as usize + 1).min(cols.len())) + } + Err(processing_error) => { + let err = processing_error("could not find range index", name_span); + return Value::Error { + error: Box::new(err), + }; } - } else { - (0usize, cols.len()) - }; - - // Merge Columns - let part1 = &cols.clone()[0..start_index]; - let combined = &cols.clone()[start_index..=end_index]; - let binding = combined.join(""); - let part3 = &cols.clone()[end_index + 1..]; - let new_cols = [part1, &[binding], part3].concat(); - // Now renumber columns since we merged some - let mut renum_cols = vec![]; - for (idx, _acol) in new_cols.iter().enumerate() { - renum_cols.push(format!("column{idx}")); - } - - // Merge Values - let part1 = &vals.clone()[0..start_index]; - let combined = &vals.clone()[start_index..=end_index]; - let binding = Value::string( - combined - .iter() - .map(|f| match f.as_string() { - Ok(s) => s, - _ => "".to_string(), - }) - .join(" "), // add a space between items - Span::unknown(), - ); - let part3 = &vals.clone()[end_index + 1..]; - let new_vals = [part1, &[binding], part3].concat(); - - Value::Record { - cols: renum_cols, - vals: new_vals, - span: name_span, } } else { - Value::Record { + return Value::Record { cols, vals, span: name_span, - } + }; + }; + + // Merge Columns + ((start_index + 1)..(cols.len() - end_index + start_index + 1)).for_each(|idx| { + cols.swap(idx, end_index - start_index - 1 + idx); + }); + cols.truncate(cols.len() - end_index + start_index + 1); + + // Merge Values + let combined = vals + .iter() + .take(end_index) + .skip(start_index) + .map(|v| v.as_string().unwrap_or(String::default())) + .join(" "); + let binding = Value::string(combined, Span::unknown()); + let last_seg = vals.split_off(end_index); + vals.truncate(start_index); + vals.push(binding); + last_seg.into_iter().for_each(|v| vals.push(v)); + + Value::Record { + cols, + vals, + span: name_span, } }) .into_pipeline_data(ctrlc)) diff --git a/crates/nu-command/tests/commands/detect_columns.rs b/crates/nu-command/tests/commands/detect_columns.rs new file mode 100644 index 0000000000..89a6bdc7eb --- /dev/null +++ b/crates/nu-command/tests/commands/detect_columns.rs @@ -0,0 +1,65 @@ +use nu_test_support::{nu, playground::Playground}; + +#[test] +fn detect_columns() { + let cases = [( + "$\"c1 c2 c3 c4 c5(char nl)a b c d e\"", + "[[c1,c2,c3,c4,c5]; [a,b,c,d,e]]", + )]; + + Playground::setup("detect_columns_test_1", |dirs, _| { + for case in cases.into_iter() { + let out = nu!( + cwd: dirs.test(), + "({} | detect columns) == {}", + case.0, + case.1 + ); + + assert_eq!( + out.out, "true", + "({} | detect columns) == {}", + case.0, case.1 + ); + } + }); +} + +#[test] +fn detect_columns_with_flag_c() { + let cases = [ + ( + "$\"c1 c2 c3 c4 c5(char nl)a b c d e\"", + "[[c1,c3,c4,c5]; ['a b',c,d,e]]", + "0..1", + ), + ( + "$\"c1 c2 c3 c4 c5(char nl)a b c d e\"", + "[[c1,c2,c3,c4]; [a,b,c,'d e']]", + "(-2)..(-1)", + ), + ( + "$\"c1 c2 c3 c4 c5(char nl)a b c d e\"", + "[[c1,c2,c3]; [a,b,'c d e']]", + "2..", + ), + ]; + + Playground::setup("detect_columns_test_1", |dirs, _| { + for case in cases.into_iter() { + let out = nu!( + cwd: dirs.test(), + "({} | detect columns -c {}) == {}", + case.0, + case.2, + case.1, + ); + + assert_eq!( + out.out, "true", + "({} | detect columns -c {}) == {}", + case.0, case.2, case.1 + ); + } + }); +} diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs index 87c95bd9fa..90d8cf21c1 100644 --- a/crates/nu-command/tests/commands/mod.rs +++ b/crates/nu-command/tests/commands/mod.rs @@ -12,6 +12,7 @@ mod cp; mod date; mod def; mod default; +mod detect_columns; mod do_; mod drop; mod each;