Simplify seq char (#7054)

* Simplify `seq char`

* Fix input/output tests

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

@@ -14,27 +14,21 @@ impl Command for SeqChar {
     }
 
     fn usage(&self) -> &str {
-        "Print sequence of chars"
+        "Print a sequence of ASCII characters"
     }
 
     fn signature(&self) -> Signature {
         Signature::build("seq char")
-            .input_output_types(vec![
+            .input_output_types(vec![(Type::Nothing, Type::List(Box::new(Type::String)))])
-                (Type::Nothing, Type::List(Box::new(Type::Any))),
+            .required(
-                (Type::Nothing, Type::String),
+                "start",
-            ])
-            .rest("rest", SyntaxShape::String, "sequence chars")
-            .named(
-                "separator",
                 SyntaxShape::String,
-                "separator character (defaults to \\n)",
+                "start of character sequence (inclusive)",
-                Some('s'),
             )
-            .named(
+            .required(
-                "terminator",
+                "end",
                 SyntaxShape::String,
-                "terminator character (defaults to \\n)",
+                "end of character sequence (inclusive)",
-                Some('t'),
             )
             .category(Category::Generators)
     }
@@ -42,7 +36,7 @@ impl Command for SeqChar {
     fn examples(&self) -> Vec<Example> {
         vec![
             Example {
-                description: "sequence a to e with newline separator",
+                description: "sequence a to e",
                 example: "seq char a e",
                 result: Some(Value::List {
                     vals: vec![
@@ -56,9 +50,10 @@ impl Command for SeqChar {
                 }),
             },
             Example {
-                description: "sequence a to e with pipe separator separator",
+                description: "sequence a to e, and put the characters in a pipe-separated string",
-                example: "seq char -s '|' a e",
+                example: "seq char a e | str join '|'",
-                result: Some(Value::test_string("a|b|c|d|e")),
+                // TODO: it would be nice to test this example, but it currently breaks the input/output type tests
+                result: None,
             },
         ]
     }
@@ -83,118 +78,58 @@ fn seq_char(
     stack: &mut Stack,
     call: &Call,
 ) -> Result<PipelineData, ShellError> {
-    // input check.
+    let start: Spanned<String> = call.req(engine_state, stack, 0)?;
-    let separator: Option<Spanned<String>> = call.get_flag(engine_state, stack, "separator")?;
+    let end: Spanned<String> = call.req(engine_state, stack, 1)?;
-    let terminator: Option<Spanned<String>> = call.get_flag(engine_state, stack, "terminator")?;
-    let rest_inputs: Vec<Spanned<String>> = call.rest(engine_state, stack, 0)?;
 
-    let (start_ch, end_ch) = if rest_inputs.len() != 2
+    if !is_single_character(&start.item) {
-        || !is_single_character(&rest_inputs[0].item)
-        || !is_single_character(&rest_inputs[1].item)
-    {
         return Err(ShellError::GenericError(
-            "seq char required two character parameters".into(),
+            "seq char only accepts individual ASCII characters as parameters".into(),
-            "needs parameter".into(),
+            "should be 1 character long".into(),
-            Some(call.head),
+            Some(start.span),
             None,
             Vec::new(),
         ));
-    } else {
+    }
-        // unwrap here is ok, because we just check the length of `rest_inputs`.
-        (
-            rest_inputs[0]
-                .item
-                .chars()
-                .next()
-                .expect("seq char input must contains 2 inputs"),
-            rest_inputs[1]
-                .item
-                .chars()
-                .next()
-                .expect("seq char input must contains 2 inputs"),
-        )
-    };
 
-    let sep: String = match separator {
+    if !is_single_character(&end.item) {
-        Some(s) => {
+        return Err(ShellError::GenericError(
-            if s.item == r"\t" {
+            "seq char only accepts individual ASCII characters as parameters".into(),
-                '\t'.to_string()
+            "should be 1 character long".into(),
-            } else if s.item == r"\n" {
+            Some(end.span),
-                '\n'.to_string()
+            None,
-            } else if s.item == r"\r" {
+            Vec::new(),
-                '\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 terminator: String = match terminator {
+    let start = start
-        Some(t) => {
+        .item
-            if t.item == r"\t" {
+        .chars()
-                '\t'.to_string()
+        .next()
-            } else if t.item == r"\n" {
+        // expect is ok here, because we just checked the length
-                '\n'.to_string()
+        .expect("seq char input must contains 2 inputs");
-            } else if t.item == r"\r" {
+
-                '\r'.to_string()
+    let end = end
-            } else {
+        .item
-                let vec_t: Vec<char> = t.item.chars().collect();
+        .chars()
-                if vec_t.is_empty() {
+        .next()
-                    return Err(ShellError::GenericError(
+        // expect is ok here, because we just checked the length
-                        "Expected a single terminator char from --terminator".into(),
+        .expect("seq char input must contains 2 inputs");
-                        "requires a single character string input".into(),
-                        Some(t.span),
-                        None,
-                        Vec::new(),
-                    ));
-                };
-                vec_t.iter().collect()
-            }
-        }
-        _ => '\n'.to_string(),
-    };
 
     let span = call.head;
-    run_seq_char(start_ch, end_ch, sep, terminator, span)
+    run_seq_char(start, end, span)
 }
 
-fn run_seq_char(
+fn run_seq_char(start_ch: char, end_ch: char, span: Span) -> Result<PipelineData, ShellError> {
-    start_ch: char,
-    end_ch: char,
-    sep: String,
-    terminator: String,
-    span: Span,
-) -> Result<PipelineData, ShellError> {
     let mut result_vec = vec![];
     for current_ch in start_ch as u8..end_ch as u8 + 1 {
         result_vec.push((current_ch as char).to_string())
     }
-    let return_list = (sep == "\n" || sep == "\r") && (terminator == "\n" || terminator == "\r");
+
-    if return_list {
+    let result = result_vec
-        let result = result_vec
+        .into_iter()
-            .into_iter()
+        .map(|x| Value::String { val: x, span })
-            .map(|x| Value::String { val: x, span })
+        .collect::<Vec<Value>>();
-            .collect::<Vec<Value>>();
+    Ok(Value::List { vals: result, span }.into_pipeline_data())
-        Ok(Value::List { vals: result, span }.into_pipeline_data())
-    } else {
-        let mut result = result_vec.join(&sep);
-        result.push_str(&terminator);
-        // doesn't output a list, if separator is '\n', it's better to eliminate them.
-        // and it matches `seq` behavior.
-        let result = result.lines().collect();
-        Ok(Value::String { val: result, span }.into_pipeline_data())
-    }
 }
 
 #[cfg(test)]

crates/nu-command/tests/commands/mod.rs

@@ -67,6 +67,7 @@ mod run_external;
 mod save;
 mod select;
 mod semicolon;
+mod seq_char;
 mod shells;
 mod skip;
 mod sort_by;

crates/nu-command/tests/commands/seq_char.rs

@@ -0,0 +1,25 @@
+use nu_test_support::{nu, pipeline};
+
+#[test]
+fn fails_when_first_arg_is_multiple_chars() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+        seq char aa z
+        "#
+    ));
+
+    assert!(actual.err.contains("should be 1 character long"));
+}
+
+#[test]
+fn fails_when_second_arg_is_multiple_chars() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+        seq char a zz
+        "#
+    ));
+
+    assert!(actual.err.contains("should be 1 character long"));
+}