Make seq output type consistent (#7045)

crates/nu-command/src/generators/seq.rs

@@ -5,7 +5,6 @@ use nu_protocol::{
     Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Spanned,
     SyntaxShape, Type, Value,
 };
-use std::cmp;
 
 #[derive(Clone)]
 pub struct Seq;
@@ -23,28 +22,11 @@ impl Command for Seq {
                 (Type::Nothing, Type::String),
             ])
             .rest("rest", SyntaxShape::Number, "sequence values")
-            .named(
-                "separator",
-                SyntaxShape::String,
-                "separator character (defaults to \\n)",
-                Some('s'),
-            )
-            .named(
-                "terminator",
-                SyntaxShape::String,
-                "terminator character (defaults to \\n)",
-                Some('t'),
-            )
-            .switch(
-                "widths",
-                "equalize widths of all numbers by padding with zeros",
-                Some('w'),
-            )
             .category(Category::Generators)
     }
 
     fn usage(&self) -> &str {
-        "Print sequences of numbers."
+        "Output sequences of numbers."
     }
 
     fn run(
@@ -60,7 +42,7 @@ impl Command for Seq {
     fn examples(&self) -> Vec<Example> {
         vec![
             Example {
-                description: "sequence 1 to 10 with newline separator",
+                description: "sequence 1 to 10",
                 example: "seq 1 10",
                 result: Some(Value::List {
                     vals: vec![
@@ -79,7 +61,7 @@ impl Command for Seq {
                 }),
             },
             Example {
-                description: "sequence 1.0 to 2.0 by 0.1s with newline separator",
+                description: "sequence 1.0 to 2.0 by 0.1s",
                 example: "seq 1.0 0.1 2.0",
                 result: Some(Value::List {
                     vals: vec![
@@ -99,19 +81,9 @@ impl Command for Seq {
                 }),
             },
             Example {
-                description: "sequence 1 to 10 with pipe separator",
-                example: "seq -s '|' 1 10",
-                result: Some(Value::test_string("1|2|3|4|5|6|7|8|9|10")),
-            },
-            Example {
-                description: "sequence 1 to 10 with pipe separator padded with 0",
-                example: "seq -s '|' -w 1 10",
-                result: Some(Value::test_string("01|02|03|04|05|06|07|08|09|10")),
-            },
-            Example {
-                description: "sequence 1 to 10 with pipe separator padded by 2s",
-                example: "seq -s ' | ' -w 1 2 10",
-                result: Some(Value::test_string("01 | 03 | 05 | 07 | 09")),
+                description: "sequence 1 to 5, then convert to a string with a pipe separator",
+                example: "seq 1 5 | str join '|'",
+                result: Some(Value::test_string("1|2|3|4|5")),
             },
         ]
     }
@@ -124,9 +96,6 @@ fn seq(
 ) -> Result<PipelineData, ShellError> {
     let span = call.head;
     let rest_nums: Vec<Spanned<f64>> = call.rest(engine_state, stack, 0)?;
-    let separator: Option<Spanned<String>> = call.get_flag(engine_state, stack, "separator")?;
-    let terminator: Option<Spanned<String>> = call.get_flag(engine_state, stack, "terminator")?;
-    let widths = call.has_flag("widths");
 
     if rest_nums.is_empty() {
         return Err(ShellError::GenericError(
@@ -138,59 +107,9 @@ fn seq(
         ));
     }
 
-    let sep: String = match separator {
-        Some(s) => {
-            if s.item == r"\t" {
-                '\t'.to_string()
-            } else if s.item == r"\n" {
-                '\n'.to_string()
-            } else if s.item == r"\r" {
-                '\r'.to_string()
-            } else {
-                let vec_s: Vec<char> = s.item.chars().collect();
-                if vec_s.is_empty() {
-                    return Err(ShellError::GenericError(
-                        "Expected a single separator char from --separator".into(),
-                        "requires a single character string input".into(),
-                        Some(s.span),
-                        None,
-                        Vec::new(),
-                    ));
-                };
-                vec_s.iter().collect()
-            }
-        }
-        _ => '\n'.to_string(),
-    };
+    let rest_nums: Vec<f64> = rest_nums.iter().map(|n| n.item).collect();
 
-    let term: String = match terminator {
-        Some(t) => {
-            if t.item == r"\t" {
-                '\t'.to_string()
-            } else if t.item == r"\n" {
-                '\n'.to_string()
-            } else if t.item == r"\r" {
-                '\r'.to_string()
-            } else {
-                let vec_t: Vec<char> = t.item.chars().collect();
-                if vec_t.is_empty() {
-                    return Err(ShellError::GenericError(
-                        "Expected a single terminator char from --terminator".into(),
-                        "requires a single character string input".into(),
-                        Some(t.span),
-                        None,
-                        Vec::new(),
-                    ));
-                };
-                vec_t.iter().collect()
-            }
-        }
-        _ => '\n'.to_string(),
-    };
-
-    let rest_nums: Vec<String> = rest_nums.iter().map(|n| n.item.to_string()).collect();
-
-    run_seq(sep, Some(term), widths, rest_nums, span)
+    run_seq(rest_nums, span)
 }
 
 #[cfg(test)]
@@ -205,107 +124,13 @@ mod tests {
     }
 }
 
-fn parse_float(mut s: &str) -> Result<f64, String> {
-    if s.starts_with('+') {
-        s = &s[1..];
-    }
-    match s.parse() {
-        Ok(n) => Ok(n),
-        Err(e) => Err(format!(
-            "seq: invalid floating point argument `{}`: {}",
-            s, e
-        )),
-    }
-}
+pub fn run_seq(free: Vec<f64>, span: Span) -> Result<PipelineData, ShellError> {
+    let first = free[0];
 
-fn escape_sequences(s: &str) -> String {
-    s.replace("\\n", "\n").replace("\\t", "\t")
-}
+    let step: f64 = if free.len() > 2 { free[1] } else { 1.0 };
+    let last = { free[free.len() - 1] };
 
-pub fn run_seq(
-    sep: String,
-    termy: Option<String>,
-    widths: bool,
-    free: Vec<String>,
-    span: Span,
-) -> Result<PipelineData, ShellError> {
-    let mut largest_dec = 0;
-    let mut padding = 0;
-    let first = if free.len() > 1 {
-        let slice = &free[0][..];
-        let len = slice.len();
-        let dec = slice.find('.').unwrap_or(len);
-        largest_dec = len - dec;
-        padding = dec;
-        match parse_float(slice) {
-            Ok(n) => n,
-            Err(s) => {
-                return Err(ShellError::GenericError(
-                    s,
-                    "".to_string(),
-                    None,
-                    Some("error parsing float".into()),
-                    Vec::new(),
-                ))
-            }
-        }
-    } else {
-        1.0
-    };
-    let step = if free.len() > 2 {
-        let slice = &free[1][..];
-        let len = slice.len();
-        let dec = slice.find('.').unwrap_or(len);
-        largest_dec = cmp::max(largest_dec, len - dec);
-        padding = cmp::max(padding, dec);
-        match parse_float(slice) {
-            Ok(n) => n,
-            Err(s) => {
-                return Err(ShellError::GenericError(
-                    s,
-                    "".to_string(),
-                    None,
-                    Some("error parsing float".into()),
-                    Vec::new(),
-                ))
-            }
-        }
-    } else {
-        1.0
-    };
-    let last = {
-        let slice = &free[free.len() - 1][..];
-        padding = cmp::max(padding, slice.find('.').unwrap_or(slice.len()));
-        match parse_float(slice) {
-            Ok(n) => n,
-            Err(s) => {
-                return Err(ShellError::GenericError(
-                    s,
-                    "".to_string(),
-                    None,
-                    Some("error parsing float".into()),
-                    Vec::new(),
-                ));
-            }
-        }
-    };
-    largest_dec = largest_dec.saturating_sub(1);
-    let separator = escape_sequences(&sep[..]);
-    let terminator = match termy {
-        Some(term) => escape_sequences(&term[..]),
-        None => separator.clone(),
-    };
-    Ok(print_seq(
-        first,
-        step,
-        last,
-        largest_dec,
-        separator,
-        terminator,
-        widths,
-        padding,
-        span,
-    ))
+    Ok(print_seq(first, step, last, span))
 }
 
 fn done_printing(next: f64, step: f64, last: f64) -> bool {
@@ -316,75 +141,33 @@ fn done_printing(next: f64, step: f64, last: f64) -> bool {
     }
 }
 
-#[allow(clippy::too_many_arguments)]
-fn print_seq(
-    first: f64,
-    step: f64,
-    last: f64,
-    largest_dec: usize,
-    separator: String,
-    terminator: String,
-    pad: bool,
-    padding: usize,
-    span: Span,
-) -> PipelineData {
+fn print_seq(first: f64, step: f64, last: f64, span: Span) -> PipelineData {
     let mut i = 0isize;
     let mut value = first + i as f64 * step;
-    // for string output
-    let mut ret_str = "".to_owned();
-    // for number output
     let mut ret_num = vec![];
-    // If the separator and terminator are line endings we can convert to numbers
-    let use_num =
-        (separator == "\n" || separator == "\r") && (terminator == "\n" || terminator == "\r");
 
     while !done_printing(value, step, last) {
-        if use_num {
-            ret_num.push(value);
-        } else {
-            // formatting for string output with potential padding
-            let istr = format!("{:.*}", largest_dec, value);
-            let ilen = istr.len();
-            let before_dec = istr.find('.').unwrap_or(ilen);
-            if pad && before_dec < padding {
-                for _ in 0..(padding - before_dec) {
-                    ret_str.push('0');
-                }
-            }
-            ret_str.push_str(&istr);
-        }
+        ret_num.push(value);
         i += 1;
         value = first + i as f64 * step;
-        if !done_printing(value, step, last) {
-            ret_str.push_str(&separator);
-        }
     }
 
-    if !use_num && ((first >= last && step < 0f64) || (first <= last && step > 0f64)) {
-        ret_str.push_str(&terminator);
-    }
+    // we'd like to keep the datatype the same for the output, so check
+    // and see if any of the output contains values after the decimal point,
+    // and if so we'll make the entire output floats
+    let contains_decimals = vec_contains_decimals(&ret_num);
+    let rows: Vec<Value> = ret_num
+        .iter()
+        .map(|v| {
+            if contains_decimals {
+                Value::float(*v, span)
+            } else {
+                Value::int(*v as i64, span)
+            }
+        })
+        .collect();
 
-    if use_num {
-        // we'd like to keep the datatype the same for the output, so check
-        // and see if any of the output is really decimals, and if it is
-        // we'll make the entire output decimals
-        let contains_decimals = vec_contains_decimals(&ret_num);
-        let rows: Vec<Value> = ret_num
-            .iter()
-            .map(|v| {
-                if contains_decimals {
-                    Value::float(*v, span)
-                } else {
-                    Value::int(*v as i64, span)
-                }
-            })
-            .collect();
-
-        Value::List { vals: rows, span }.into_pipeline_data()
-    } else {
-        let rows: String = ret_str.lines().collect();
-        Value::string(rows, span).into_pipeline_data()
-    }
+    Value::List { vals: rows, span }.into_pipeline_data()
 }
 
 fn vec_contains_decimals(array: &[f64]) -> bool {

src/tests/test_parser.rs

@@ -198,7 +198,10 @@ fn commands_have_usage() -> TestResult {
 
 #[test]
 fn equals_separates_long_flag() -> TestResult {
-    run_test(r#"seq 1 4 --separator='+'"#, "1+2+3+4")
+    run_test(
+        r#"'nushell' | str lpad --length=10 --character='-'"#,
+        "---nushell",
+    )
 }
 
 #[test]