From 97e7d550c81fa3a131fb362a7a29d860625dacc0 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Wed, 29 Mar 2023 20:01:42 +1300 Subject: [PATCH] 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. --- .../nu-command/src/strings/str_/substring.rs | 135 +++--------------- crates/nu-command/tests/commands/str_/mod.rs | 12 +- src/tests/test_strings.rs | 2 +- 3 files changed, 29 insertions(+), 120 deletions(-) diff --git a/crates/nu-command/src/strings/str_/substring.rs b/crates/nu-command/src/strings/str_/substring.rs index ebdf37436e..93e7400264 100644 --- a/crates/nu-command/src/strings/str_/substring.rs +++ b/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 { - 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 = 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())) +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, + }) } - Value::List { vals, .. } => { - if vals.len() > 2 { - Err(ShellError::TypeMismatch { - err_message: "More than two indices given".to_string(), - span: head, - }) - } else { - let idx: Vec = 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 { - err_message: "could not perform substring".to_string(), - span: head, - })?, + 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)) diff --git a/crates/nu-command/tests/commands/str_/mod.rs b/crates/nu-command/tests/commands/str_/mod.rs index 6d5c5c62cb..b18b29452b 100644 --- a/crates/nu-command/tests/commands/str_/mod.rs +++ b/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 "# )); diff --git a/src/tests/test_strings.rs b/src/tests/test_strings.rs index f93420e721..57378977a2 100644 --- a/src/tests/test_strings.rs +++ b/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", ) }