From d649c2aab4932295cb067383ab21db6af982e56c Mon Sep 17 00:00:00 2001 From: Nahor Date: Tue, 7 Apr 2026 11:20:49 -0700 Subject: [PATCH] string: remove `StringError::NotANumber` Use the more generic `StringError::InvalidArgs` instead Closes #12556 --- src/builtins/string.rs | 33 ++++++++++----------------------- src/builtins/string/pad.rs | 2 +- src/builtins/string/repeat.rs | 4 ++-- src/builtins/string/shorten.rs | 2 +- src/builtins/string/split.rs | 2 +- src/builtins/string/sub.rs | 6 +++--- 6 files changed, 18 insertions(+), 31 deletions(-) diff --git a/src/builtins/string.rs b/src/builtins/string.rs index 1761188e5..0f2d5b981 100644 --- a/src/builtins/string.rs +++ b/src/builtins/string.rs @@ -1,4 +1,4 @@ -use crate::{builtins::error::Error, err_fmt, err_raw, err_str, screen::escape_code_length}; +use crate::{err_fmt, err_raw, err_str, screen::escape_code_length}; use fish_wcstringutil::fish_wcwidth_visible; // Forward some imports to make subcmd implementations easier use super::prelude::*; @@ -69,7 +69,7 @@ fn parse_opts( c => { let retval = self.parse_opt(c, w.woptarg); if let Err(e) = retval { - e.print_error(&args_read, streams, w.woptarg, w.wopt_index); + e.print_error(&args_read, streams, w.wopt_index); return Err(STATUS_INVALID_ARGS); } } @@ -79,6 +79,11 @@ fn parse_opts( Ok(w.wopt_index) } + fn parse_arg_number<'a, 'b>(arg: &'a wstr) -> Result> { + let n = fish_wcstol(arg).map_err(|_| err_fmt!(Error::NOT_NUMBER, arg))?; + Ok(n) + } + /// Take any positional arguments after options have been parsed. #[allow(unused_variables)] fn take_args( @@ -143,7 +148,6 @@ fn run_impl( /// This covers failing argument/option parsing enum StringError<'a> { InvalidArgs(Error<'a>), - NotANumber, UnknownOption, } @@ -190,26 +194,14 @@ fn print_error(&self, args: &[&wstr], streams: &mut IoStreams) { } } -impl<'a> From for StringError<'a> { - fn from(_: crate::wutil::wcstoi::Error) -> Self { - StringError::NotANumber - } -} - -impl<'a> From> for StringError<'a> { - fn from(error: crate::builtins::error::Error<'a>) -> Self { +impl<'a> From> for StringError<'a> { + fn from(error: error::Error<'a>) -> Self { StringError::InvalidArgs(error) } } impl<'a> StringError<'a> { - fn print_error( - self, - args: &[&wstr], - streams: &mut IoStreams, - optarg: Option<&wstr>, - optind: usize, - ) { + fn print_error(self, args: &[&wstr], streams: &mut IoStreams, optind: usize) { let cmd = L!("string"); let subcmd = args[0]; use StringError::*; @@ -217,11 +209,6 @@ fn print_error( InvalidArgs(err) => { err.subcmd(cmd, subcmd).finish(streams); } - NotANumber => { - err_fmt!(Error::NOT_NUMBER, optarg.unwrap()) - .subcmd(cmd, subcmd) - .finish(streams); - } UnknownOption => { // This would mean the subcmd's XXX_OPTIONS does not match // the list in its `parse_opt()` implementation diff --git a/src/builtins/string/pad.rs b/src/builtins/string/pad.rs index 0fabf9d4f..bbcf069cb 100644 --- a/src/builtins/string/pad.rs +++ b/src/builtins/string/pad.rs @@ -52,7 +52,7 @@ fn parse_opt(&mut self, c: char, arg: Option<&wstr>) -> Result<(), StringError<' 'r' => self.pad_from = Direction::Right, 'w' => { let arg = arg.unwrap(); - self.width = fish_wcstol(arg)? + self.width = Self::parse_arg_number(arg)? .try_into() .map_err(|_| err_fmt!("Invalid width value '%s'", arg))?; } diff --git a/src/builtins/string/repeat.rs b/src/builtins/string/repeat.rs index 44fcf664e..f57b5e9b6 100644 --- a/src/builtins/string/repeat.rs +++ b/src/builtins/string/repeat.rs @@ -22,7 +22,7 @@ fn parse_opt(&mut self, c: char, arg: Option<&wstr>) -> Result<(), StringError<' 'n' => { let arg = arg.unwrap(); self.count = Some( - fish_wcstol(arg)? + Self::parse_arg_number(arg)? .try_into() .map_err(|_| err_fmt!("Invalid count value '%s'", arg))?, ); @@ -30,7 +30,7 @@ fn parse_opt(&mut self, c: char, arg: Option<&wstr>) -> Result<(), StringError<' 'm' => { let arg = arg.unwrap(); self.max = Some( - fish_wcstol(arg)? + Self::parse_arg_number(arg)? .try_into() .map_err(|_| err_fmt!(Error::INVALID_MAX_VALUE, arg))?, ); diff --git a/src/builtins/string/shorten.rs b/src/builtins/string/shorten.rs index 02592f497..06b9d7b3c 100644 --- a/src/builtins/string/shorten.rs +++ b/src/builtins/string/shorten.rs @@ -46,7 +46,7 @@ fn parse_opt(&mut self, c: char, arg: Option<&'args wstr>) -> Result<(), StringE 'm' => { let arg = arg.unwrap(); self.max = Some( - fish_wcstol(arg)? + Self::parse_arg_number(arg)? .try_into() .map_err(|_| err_fmt!(Error::INVALID_MAX_VALUE, arg))?, ); diff --git a/src/builtins/string/split.rs b/src/builtins/string/split.rs index fa589c2b7..149afb685 100644 --- a/src/builtins/string/split.rs +++ b/src/builtins/string/split.rs @@ -118,7 +118,7 @@ fn parse_opt(&mut self, c: char, arg: Option<&wstr>) -> Result<(), StringError<' 'r' => self.split_from = Direction::Right, 'm' => { let arg = arg.unwrap(); - self.max = fish_wcstol(arg)? + self.max = Self::parse_arg_number(arg)? .try_into() .map_err(|_| err_fmt!(Error::INVALID_MAX_VALUE, arg))?; } diff --git a/src/builtins/string/sub.rs b/src/builtins/string/sub.rs index 0d4a6ebcb..e2a581855 100644 --- a/src/builtins/string/sub.rs +++ b/src/builtins/string/sub.rs @@ -24,7 +24,7 @@ fn parse_opt(&mut self, c: char, arg: Option<&wstr>) -> Result<(), StringError<' 'l' => { let arg = arg.unwrap(); self.length = Some( - fish_wcstol(arg)? + Self::parse_arg_number(arg)? .try_into() .map_err(|_| err_fmt!("Invalid length value '%s'", arg))?, ); @@ -32,7 +32,7 @@ fn parse_opt(&mut self, c: char, arg: Option<&wstr>) -> Result<(), StringError<' 's' => { let arg = arg.unwrap(); self.start = Some( - fish_wcstol(arg)? + Self::parse_arg_number(arg)? .try_into() .map_err(|_| err_fmt!("Invalid start value '%s'", arg))?, ); @@ -40,7 +40,7 @@ fn parse_opt(&mut self, c: char, arg: Option<&wstr>) -> Result<(), StringError<' 'e' => { let arg = arg.unwrap(); self.end = Some( - fish_wcstol(arg)? + Self::parse_arg_number(arg)? .try_into() .map_err(|_| err_fmt!("Invalid end value '%s'", arg))?, );