fix #9653 the cmd detect columns with the flag -c (#9667)

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 <Int> - number of rows to skip before detecting
  -n, --no-headers - don't detect headers
  -c, --combine-columns <Range> - columns to be combined; listed as a range

Signatures:
  <string> | detect columns -> <table>

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

```

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() {
+            let (start_index, end_index) = if let Some(range) = &range {
-                // Destructure the range parameter
+                match nu_cmd_base::util::process_range(range) {
-                let (start_index, end_index) = if let Some(range) = &range {
+                    Ok((l_idx, r_idx)) => {
-                    match nu_cmd_base::util::process_range(range) {
+                        let l_idx = if l_idx < 0 {
-                        Ok(r) => {
+                            cols.len() as isize + l_idx
-                            // `process_range()` returns `isize::MAX` if the range is open-ended,
+                        } else {
-                            // which is not ideal for us
+                            l_idx
-                            let end = if r.1 as usize > cols.len() {
+                        };
-                                cols.len()
+
-                            } else {
+                        let r_idx = if r_idx < 0 {
-                                r.1 as usize
+                            cols.len() as isize + r_idx
-                            };
+                        } else {
-                            (r.0 as usize, end)
+                            r_idx
-                        }
+                        };
-                        Err(processing_error) => {
+
-                            let err = processing_error("could not find range index", name_span);
+                        if !(l_idx <= r_idx && (r_idx >= 0 || l_idx < (cols.len() as isize))) {
-                            return Value::Error {
+                            return Value::Record {
-                                error: Box::new(err),
+                                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))

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
+            );
+        }
+    });
+}

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;