move 'str substring' to only use ranges (#8660)

# Description

This removes all the old style of quasi-ranges before we had full range
support from `str substring`. Functionality should otherwise work, but
only with the official range syntax.

# User-Facing Changes

Removes the array and string forms of ranges from `str substring`.
Leaves only the official range support for range values.

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.

crates/nu-command/src/strings/str_/substring.rs

@@ -4,7 +4,9 @@ use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::ast::CellPath;
 use nu_protocol::engine::{Command, EngineState, Stack};
-use nu_protocol::{Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value};
+use nu_protocol::{
+    Example, PipelineData, Range, ShellError, Signature, Span, SyntaxShape, Type, Value,
+};
 use std::cmp::Ordering;
 use unicode_segmentation::UnicodeSegmentation;
 
@@ -32,8 +34,6 @@ impl From<(isize, isize)> for Substring {
     }
 }
 
-struct SubstringText(String, String);
-
 impl Command for SubCommand {
     fn name(&self) -> &str {
         "str substring"
@@ -80,7 +80,7 @@ impl Command for SubCommand {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        let range = call.req(engine_state, stack, 0)?;
+        let range: Range = call.req(engine_state, stack, 0)?;
         let indexes: Substring = process_arguments(&range, call.head)?.into();
         let cell_paths: Vec<CellPath> = call.rest(engine_state, stack, 1)?;
         let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
@@ -100,31 +100,6 @@ impl Command for SubCommand {
                 example: " 'good nushell' | str substring 5..12",
                 result: Some(Value::test_string("nushell")),
             },
-            Example {
-                description: "Alternately, you can pass in a list",
-                example: " 'good nushell' | str substring [5 12]",
-                result: Some(Value::test_string("nushell")),
-            },
-            Example {
-                description: "Or a simple comma-separated string",
-                example: " 'good nushell' | str substring '5,12'",
-                result: Some(Value::test_string("nushell")),
-            },
-            Example {
-                description: "Drop the last `n` characters from the string",
-                example: " 'good nushell' | str substring ',-5'",
-                result: Some(Value::test_string("good nu")),
-            },
-            Example {
-                description: "Get the remaining characters from a starting index",
-                example: " 'good nushell' | str substring '5,'",
-                result: Some(Value::test_string("nushell")),
-            },
-            Example {
-                description: "Get the characters from the beginning until ending index",
-                example: " 'good nushell' | str substring ',7'",
-                result: Some(Value::test_string("good nu")),
-            },
             Example {
                 description: "Count indexes and split using grapheme clusters",
                 example: " '🇯🇵ほげ ふが ぴよ' | str substring -g 4..6",
@@ -211,93 +186,27 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value {
     }
 }
 
-fn process_arguments(range: &Value, head: Span) -> Result<(isize, isize), ShellError> {
-    let search = match range {
-        Value::Range { val, .. } => {
-            let start = val.from()?;
-            let end = val.to()?;
-            Ok(SubstringText(start.to_string(), end.to_string()))
-        }
-        Value::List { vals, .. } => {
-            if vals.len() > 2 {
-                Err(ShellError::TypeMismatch {
-                    err_message: "More than two indices given".to_string(),
+fn process_arguments(range: &Range, head: Span) -> Result<(isize, isize), ShellError> {
+    let start = match &range.from {
+        Value::Int { val, .. } => *val as isize,
+        Value::Nothing { .. } => 0,
+        _ => {
+            return Err(ShellError::TypeMismatch {
+                err_message: "could not perform substring".to_string(),
                 span: head,
             })
-            } else {
-                let idx: Vec<String> = vals
-                    .iter()
-                    .map(|v| {
-                        match v {
-                            Value::Int { val, .. } => Ok(val.to_string()),
-                            Value::String { val, .. } => Ok(val.to_string()),
-                            _ => Err(ShellError::TypeMismatch {
-                                err_message:
-                                    "could not perform substring. Expecting a string or int"
-                                        .to_string(),
-                                span: head,
-                            }),
         }
-                        .unwrap_or_else(|_| String::from(""))
-                    })
-                    .collect();
-
-                let start = idx
-                    .get(0)
-                    .ok_or_else(|| ShellError::TypeMismatch {
-                        err_message: "could not perform substring".to_string(),
-                        span: head,
-                    })?
-                    .to_string();
-                let end = idx
-                    .get(1)
-                    .ok_or_else(|| ShellError::TypeMismatch {
-                        err_message: "could not perform substring".to_string(),
-                        span: head,
-                    })?
-                    .to_string();
-                Ok(SubstringText(start, end))
-            }
-        }
-        Value::String { val, .. } => {
-            let idx: Vec<&str> = val.split(',').collect();
-
-            let start = idx
-                .first()
-                .ok_or_else(|| ShellError::TypeMismatch {
-                    err_message: "could not perform substring".to_string(),
-                    span: head,
-                })?
-                .to_string();
-            let end = idx
-                .get(1)
-                .ok_or_else(|| ShellError::TypeMismatch {
-                    err_message: "could not perform substring".to_string(),
-                    span: head,
-                })?
-                .to_string();
-
-            Ok(SubstringText(start, end))
-        }
-        _ => Err(ShellError::TypeMismatch {
-            err_message: "could not perform substring".to_string(),
-            span: head,
-        }),
-    }?;
-    let start = match &search {
-        SubstringText(start, _) if start.is_empty() || start == "_" => 0,
-        SubstringText(start, _) => start.trim().parse().map_err(|_| ShellError::TypeMismatch {
-            err_message: "could not perform substring".to_string(),
-            span: head,
-        })?,
     };
 
-    let end = match &search {
-        SubstringText(_, end) if end.is_empty() || end == "_" => isize::max_value(),
-        SubstringText(_, end) => end.trim().parse().map_err(|_| ShellError::TypeMismatch {
+    let end = match &range.to {
+        Value::Int { val, .. } => *val as isize,
+        Value::Nothing { .. } => isize::max_value(),
+        _ => {
+            return Err(ShellError::TypeMismatch {
                 err_message: "could not perform substring".to_string(),
                 span: head,
-        })?,
+            })
+        }
     };
 
     Ok((start, end))

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

@@ -234,7 +234,7 @@ fn substrings_the_input() {
             cwd: dirs.test(), pipeline(
             r#"
                  open sample.toml
-                 | str substring '6,14' fortune.teller.phone
+                 | str substring 6..14 fortune.teller.phone
                  | get fortune.teller.phone
              "#
         ));
@@ -258,7 +258,7 @@ fn substring_errors_if_start_index_is_greater_than_end_index() {
             cwd: dirs.test(), pipeline(
             r#"
                  open sample.toml
-                 | str substring '6,5' fortune.teller.phone
+                 | str substring 6..5 fortune.teller.phone
              "#
         ));
 
@@ -283,7 +283,7 @@ fn substrings_the_input_and_returns_the_string_if_end_index_exceeds_length() {
             cwd: dirs.test(), pipeline(
             r#"
                  open sample.toml
-                 | str substring '0,999' package.name
+                 | str substring 0..999 package.name
                  | get package.name
              "#
         ));
@@ -307,7 +307,7 @@ fn substrings_the_input_and_returns_blank_if_start_index_exceeds_length() {
             cwd: dirs.test(), pipeline(
             r#"
                  open sample.toml
-                 | str substring '50,999' package.name
+                 | str substring 50..999 package.name
                  | get package.name
              "#
         ));
@@ -331,7 +331,7 @@ fn substrings_the_input_and_treats_start_index_as_zero_if_blank_start_index_give
             cwd: dirs.test(), pipeline(
             r#"
                  open sample.toml
-                 | str substring ',2' package.name
+                 | str substring ..2 package.name
                  | get package.name
              "#
         ));
@@ -355,7 +355,7 @@ fn substrings_the_input_and_treats_end_index_as_length_if_blank_end_index_given(
             cwd: dirs.test(), pipeline(
             r#"
                  open sample.toml
-                 | str substring '3,' package.name
+                 | str substring 3.. package.name
                  | get package.name
              "#
         ));

src/tests/test_strings.rs

@@ -3,7 +3,7 @@ use crate::tests::{fail_test, run_test, TestResult};
 #[test]
 fn cjk_in_substrings() -> TestResult {
     run_test(
-        r#"let s = '[Rust 程序设计语言](title-page.md)'; let start = ($s | str index-of '('); let end = ($s | str index-of ')'); echo ($s | str substring $"($start + 1),($end)")"#,
+        r#"let s = '[Rust 程序设计语言](title-page.md)'; let start = ($s | str index-of '('); let end = ($s | str index-of ')'); $s | str substring ($start + 1)..($end)"#,
         "title-page.md",
     )
 }