From f5e1f5442273a20716033f8542a14968a34c7206 Mon Sep 17 00:00:00 2001 From: Tomasz Kramkowski Date: Wed, 2 Oct 2024 01:29:42 +0100 Subject: [PATCH 1/8] Remove #![feature(bench_black_box)] "The feature `bench_black_box` has been stable since 1.66.0 and no longer requires an attribute to enable" --- bench/examples/ridiculous_argument.rs | 2 -- bench/examples/ridiculous_flags.rs | 2 -- bench/examples/vs.rs | 2 -- 3 files changed, 6 deletions(-) diff --git a/bench/examples/ridiculous_argument.rs b/bench/examples/ridiculous_argument.rs index a40c2b2..01cbbd6 100644 --- a/bench/examples/ridiculous_argument.rs +++ b/bench/examples/ridiculous_argument.rs @@ -1,5 +1,3 @@ -#![feature(bench_black_box)] - use std::hint::black_box; use getargs::Argument; diff --git a/bench/examples/ridiculous_flags.rs b/bench/examples/ridiculous_flags.rs index 0693978..9cfbd8f 100644 --- a/bench/examples/ridiculous_flags.rs +++ b/bench/examples/ridiculous_flags.rs @@ -1,5 +1,3 @@ -#![feature(bench_black_box)] - use getargs::Options; use std::hint::black_box; diff --git a/bench/examples/vs.rs b/bench/examples/vs.rs index d2c46f3..561e794 100644 --- a/bench/examples/vs.rs +++ b/bench/examples/vs.rs @@ -1,5 +1,3 @@ -#![feature(bench_black_box)] - use bench::{ARGS, ARGS_BYTES}; use getargs::Argument; use std::hint::black_box; From 2ac356537cab923e8829b859b4c31308b3b00b12 Mon Sep 17 00:00:00 2001 From: Tomasz Kramkowski Date: Tue, 1 Oct 2024 23:33:41 +0100 Subject: [PATCH 2/8] Remove unnecessary State::ShortOptionCluster field --- src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4b88031..a945265 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -418,7 +418,7 @@ enum State { /// We are in the middle of a cluster of short options. From here, /// we can get the next short option, or we can get the value for /// the last short option. We may not get a positional argument. - ShortOptionCluster(Opt, A), + ShortOptionCluster(A), /// We just consumed a long option with a value attached with `=`, /// e.g. `--execute=expression`. We must get the following value. LongOptionWithValue(Opt, A), @@ -489,7 +489,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { let opt = Opt::Short(opt); if let Some(rest) = rest { - self.state = State::ShortOptionCluster(opt, rest); + self.state = State::ShortOptionCluster(rest); } else { self.state = State::EndOfOption(opt); } @@ -501,12 +501,12 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { } } - State::ShortOptionCluster(_, rest) => { + State::ShortOptionCluster(rest) => { let (opt, rest) = rest.consume_short_opt(); let opt = Opt::Short(opt); if let Some(rest) = rest { - self.state = State::ShortOptionCluster(opt, rest); + self.state = State::ShortOptionCluster(rest); } else { self.state = State::EndOfOption(opt); } @@ -616,7 +616,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { } } - State::ShortOptionCluster(_, val) => { + State::ShortOptionCluster(val) => { self.state = State::Start { ended_opts: false }; Ok(val.consume_short_val()) } @@ -679,7 +679,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { // If the option had no explicit `=value`, return None State::EndOfOption(_) => None, - State::ShortOptionCluster(_, val) | State::LongOptionWithValue(_, val) => { + State::ShortOptionCluster(val) | State::LongOptionWithValue(_, val) => { self.state = State::Start { ended_opts: false }; Some(val) } From b0c5a1b083dd8fe24b4cb916ff277b0fb208de55 Mon Sep 17 00:00:00 2001 From: Tomasz Kramkowski Date: Tue, 1 Oct 2024 23:35:36 +0100 Subject: [PATCH 3/8] Remove unnecessary placeholder lifetimes --- src/lib.rs | 8 ++++---- src/traits.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a945265..3c26ed3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -562,7 +562,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { /// # /// # assert_eq!(opts.is_empty(), true); /// ``` - pub fn next_arg(&'_ mut self) -> Result>> { + pub fn next_arg(&mut self) -> Result>> { if !self.opts_ended() { if let Some(opt) = self.next_opt()? { return Ok(Some(opt.into())); @@ -600,7 +600,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { /// assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('c')))); /// assert_eq!(opts.value(), Ok("see")); /// ``` - pub fn value(&'_ mut self) -> Result { + pub fn value(&mut self) -> Result { match self.state { State::Start { .. } | State::Positional(_) | State::End { .. } => { panic!("called Options::value() with no previous option") @@ -670,7 +670,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { /// assert_eq!(opts.value_opt(), Some("value")); /// assert_eq!(opts.next_opt(), Ok(None)); /// ``` - pub fn value_opt(&'_ mut self) -> Option { + pub fn value_opt(&mut self) -> Option { match self.state { State::Start { .. } | State::Positional(_) | State::End { .. } => { panic!("called Options::value_opt() with no previous option") @@ -721,7 +721,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { /// assert_eq!(opts.next_positional(), Some("bar")); /// assert_eq!(opts.next_positional(), None); /// ``` - pub fn next_positional(&'_ mut self) -> Option { + pub fn next_positional(&mut self) -> Option { match self.state { State::Start { ended_opts } => self.iter.next().or_else(|| { self.state = State::End { ended_opts }; diff --git a/src/traits.rs b/src/traits.rs index b9f8818..17af1a8 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -115,7 +115,7 @@ pub trait Argument: Copy + Eq + Debug { fn consume_short_val(self) -> Self; } -impl Argument for &'_ str { +impl Argument for &str { type ShortOpt = char; #[inline] @@ -160,7 +160,7 @@ impl Argument for &'_ str { } } -impl Argument for &'_ [u8] { +impl Argument for &[u8] { type ShortOpt = u8; #[inline] From ae2bb9e53dec553476976e7b2653f57e9b32dd21 Mon Sep 17 00:00:00 2001 From: Tomasz Kramkowski Date: Tue, 1 Oct 2024 23:36:16 +0100 Subject: [PATCH 4/8] Improve error messages This is definitely a nitpick and mostly my own opinion. "option requires a value: ..." makes it sound like the option requires itself "option does not require a value: ..." makes it sound like the value is optional Since the text will always be something the programmer has agreed is an acceptable option. It should be fine to just embed it in the middle of the message. --- src/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/error.rs b/src/error.rs index e6f7bce..abe7423 100644 --- a/src/error.rs +++ b/src/error.rs @@ -42,9 +42,9 @@ pub enum Error { impl<'arg, S: Display, A: Argument + Display + 'arg> Display for Error { fn fmt(&self, f: &mut Formatter) -> core::fmt::Result { match self { - Error::RequiresValue(opt) => write!(f, "option requires a value: {}", opt), + Error::RequiresValue(opt) => write!(f, "option '{}' requires a value", opt), Error::DoesNotRequireValue(opt) => { - write!(f, "option does not require a value: {}", opt) + write!(f, "option '{}' does not expect a value", opt) } } } From 5d3f623f6b7fc7981a6c0a0e273000982cbec5b8 Mon Sep 17 00:00:00 2001 From: Tomasz Kramkowski Date: Tue, 1 Oct 2024 23:38:35 +0100 Subject: [PATCH 5/8] Optionally ignore negative numbers as options This emulates the behaviour of python's argparse when you specify options that look like negative numbers in your parser (e.g. -1 or -.5 or -123.456 which are not all short options but argparse is versatile). This is useful if you take a optionally negative number as a positional argument. The default is to keep the normal behaviour with the alternative optionally enabled by calling Options::allow_number_short_options(..., false) --- README.md | 1 + bench/examples/ridiculous_argument.rs | 4 +- src/lib.rs | 10 ++- src/traits.rs | 87 ++++++++++++++++++++++++--- 4 files changed, 91 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 196194d..776a485 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ you do with it. * Required or optional explicit values `-eVALUE` and `--explicit=VALUE` * Positional arguments and `--` * Parse options at the beginning of the argument list, or anywhere +* Optionally parse negative numbers as positional arguments ## Benefits diff --git a/bench/examples/ridiculous_argument.rs b/bench/examples/ridiculous_argument.rs index 01cbbd6..2acc156 100644 --- a/bench/examples/ridiculous_argument.rs +++ b/bench/examples/ridiculous_argument.rs @@ -33,7 +33,7 @@ fn long_flag_value() -> Option<(&'static str, Option<&'static str>)> { #[inline(never)] fn short_flag_cluster() -> Option<&'static str> { - black_box("-verylongshortflagcluster").parse_short_cluster() + black_box("-verylongshortflagcluster").parse_short_cluster(true) } #[inline(never)] @@ -78,7 +78,7 @@ fn bytes_long_flag_value() -> Option<(&'static [u8], Option<&'static [u8]>)> { #[inline(never)] fn bytes_short_flag_cluster() -> Option<&'static [u8]> { - black_box(b"-verylongshortflagcluster").parse_short_cluster() + black_box(b"-verylongshortflagcluster").parse_short_cluster(true) } #[inline(never)] diff --git a/src/lib.rs b/src/lib.rs index 3c26ed3..0f2616b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,6 +22,7 @@ //! `--explicit=VALUE` //! * Positional arguments and `--` //! * Parse options at the beginning of the argument list, or anywhere +//! * Optionally parse negative numbers as positional arguments //! //! ## Benefits //! @@ -398,6 +399,8 @@ pub struct Options> { iter: I, /// State information. state: State, + /// Parse short option clusters which look like negative numbers as options + allow_number_sopts: bool, } #[derive(Copy, Clone, Debug)] @@ -438,9 +441,14 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { Options { iter, state: State::Start { ended_opts: false }, + allow_number_sopts: true, } } + pub fn allow_number_short_options(&mut self, value: bool) { + self.allow_number_sopts = value + } + /// Retrieves the next option. /// /// Returns `Ok(None)` if there are no more options, or `Err(..)` if @@ -484,7 +492,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { } Ok(Some(opt)) - } else if let Some(cluster) = arg.parse_short_cluster() { + } else if let Some(cluster) = arg.parse_short_cluster(self.allow_number_sopts) { let (opt, rest) = cluster.consume_short_opt(); let opt = Opt::Short(opt); diff --git a/src/traits.rs b/src/traits.rs index 17af1a8..a1a69fa 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -81,11 +81,14 @@ pub trait Argument: Copy + Eq + Debug { /// /// ``` /// # use getargs::Argument; - /// assert_eq!("-abc".parse_short_cluster(), Some("abc")); - /// assert_eq!("-a".parse_short_cluster(), Some("a")); - /// assert_eq!("-".parse_short_cluster(), None); + /// assert_eq!("-abc".parse_short_cluster(true), Some("abc")); + /// assert_eq!("-a".parse_short_cluster(true), Some("a")); + /// assert_eq!("-".parse_short_cluster(true), None); + /// assert_eq!("-1".parse_short_cluster(true), Some("1")); + /// assert_eq!("-1".parse_short_cluster(false), None); + /// assert_eq!("-a".parse_short_cluster(false), Some("a")); /// ``` - fn parse_short_cluster(self) -> Option; + fn parse_short_cluster(self, allow_number: bool) -> Option; /// Attempts to consume one short option from a "short option /// cluster", as defined by @@ -115,6 +118,34 @@ pub trait Argument: Copy + Eq + Debug { fn consume_short_val(self) -> Self; } +#[inline] +fn is_number(bytes: &[u8]) -> bool { + if bytes.is_empty() { + return false; + } + #[derive(PartialEq)] + enum State { + Integer, + Separator, + Fractional, + } + let mut state = State::Integer; + for b in bytes { + state = match state { + State::Integer => match b { + b'.' => State::Separator, + b'0'..=b'9' => State::Integer, + _ => return false, + }, + State::Separator | State::Fractional => match b { + b'0'..=b'9' => State::Fractional, + _ => return false, + }, + } + } + state != State::Separator +} + impl Argument for &str { type ShortOpt = char; @@ -138,8 +169,9 @@ impl Argument for &str { } #[inline] - fn parse_short_cluster(self) -> Option { - self.strip_prefix('-').filter(|s| !s.is_empty()) + fn parse_short_cluster(self, allow_number: bool) -> Option { + self.strip_prefix('-') + .filter(|s| !s.is_empty() && (allow_number || !is_number(s.as_ref()))) } #[inline] @@ -184,8 +216,9 @@ impl Argument for &[u8] { } #[inline] - fn parse_short_cluster(self) -> Option { - self.strip_prefix(b"-").filter(|a| !a.is_empty()) + fn parse_short_cluster(self, allow_number: bool) -> Option { + self.strip_prefix(b"-") + .filter(|a| !a.is_empty() && (allow_number || !is_number(a))) } #[inline] @@ -202,3 +235,41 @@ impl Argument for &[u8] { self } } + +#[cfg(test)] +mod tests { + use super::is_number; + #[test] + fn test_is_number() { + assert!(is_number(b"123")); + assert!(is_number(b"123.456")); + assert!(is_number(b".123")); + assert!(is_number(b"1")); + assert!(is_number(b".1")); + assert!(is_number(b"0")); + assert!(is_number(b"1")); + assert!(is_number(b"2")); + assert!(is_number(b"3")); + assert!(is_number(b"4")); + assert!(is_number(b"5")); + assert!(is_number(b"6")); + assert!(is_number(b"7")); + assert!(is_number(b"8")); + assert!(is_number(b"9")); + assert!(is_number(b"0123456789")); + } + + #[test] + fn test_not_is_number() { + assert!(!is_number(b"")); + assert!(!is_number(b"a")); + assert!(!is_number(b"0a")); + assert!(!is_number(b"a0")); + assert!(!is_number(b"1..2")); + assert!(!is_number(b"1.")); + assert!(!is_number(b"123.")); + assert!(!is_number(b"123..456")); + assert!(!is_number(b"1234b")); + assert!(!is_number(b"asdf1234foo")); + } +} From b0715c058bc0bfdb0b93848e8bc38dfa20794ae9 Mon Sep 17 00:00:00 2001 From: Tomasz Kramkowski Date: Wed, 2 Oct 2024 00:20:25 +0100 Subject: [PATCH 6/8] Add display function to Argument trait This is an idea stolen from rust-lang/rust#120048. It's useful for the next commit. --- src/traits.rs | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/traits.rs b/src/traits.rs index a1a69fa..f7528ab 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -1,4 +1,4 @@ -use core::fmt::Debug; +use core::fmt::{Debug, Display, Formatter, Write}; /// The argument trait for types that can be parsed by /// [`Options`][crate::Options]. @@ -116,6 +116,16 @@ pub trait Argument: Copy + Eq + Debug { /// [`parse_short_cluster`][Self::parse_short_cluster]. Returns the /// value that was consumed. fn consume_short_val(self) -> Self; + + /// Returns an object implementing Display which can be used to format the + /// Argument. + /// + /// # Example + /// + /// ``` + /// # use getargs::Argument; + /// assert_eq!(b"abc".display().to_string(), "abc"); + fn display(self) -> impl Display; } #[inline] @@ -190,6 +200,29 @@ impl Argument for &str { fn consume_short_val(self) -> Self { self } + + #[inline] + fn display(self) -> impl Display { + self + } +} + +struct DisplaySliceU8<'a> { + slice: &'a [u8], +} + +impl Display for DisplaySliceU8<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + f.write_str("")?; + for chunk in self.slice.utf8_chunks() { + f.write_str(chunk.valid())?; + let invalid = chunk.invalid(); + if !invalid.is_empty() { + f.write_char(char::REPLACEMENT_CHARACTER)?; + } + } + Ok(()) + } } impl Argument for &[u8] { @@ -234,6 +267,11 @@ impl Argument for &[u8] { fn consume_short_val(self) -> Self { self } + + #[inline] + fn display(self) -> impl Display { + DisplaySliceU8 { slice: self } + } } #[cfg(test)] From 574e253d834c7db356b9606134b4794bc80fe18b Mon Sep 17 00:00:00 2001 From: Tomasz Kramkowski Date: Wed, 2 Oct 2024 00:20:09 +0100 Subject: [PATCH 7/8] Forced UTF-8 options Instead of letting you choose what type you want to get back from the API, just make it a `&str`. This means that options can only ever be something which can be turned into valid UTF-8. This sounds restrictive but for users of `&str as Argument` it makes no difference and for the few users of `&[u8] as Argument` it simplifies a lot of things. It also makes `&[u8]` usable on windows via `OsStr::as_encoded_bytes` and makes `&OsStr as Argument` a possibility. --- bench/examples/ridiculous_argument.rs | 24 +++--- bench/examples/vs.rs | 24 +++--- bench/src/evolution.rs | 24 +++--- examples/complete.rs | 14 ++-- examples/os_str.rs | 2 +- examples/subcommands.rs | 12 +-- src/arg.rs | 20 ++--- src/error.rs | 25 ++++-- src/iter.rs | 10 +-- src/lib.rs | 40 +++++----- src/opt.rs | 12 +-- src/tests.rs | 16 ++-- src/traits.rs | 105 ++++++++++++++++++-------- 13 files changed, 194 insertions(+), 134 deletions(-) diff --git a/bench/examples/ridiculous_argument.rs b/bench/examples/ridiculous_argument.rs index 2acc156..3f22734 100644 --- a/bench/examples/ridiculous_argument.rs +++ b/bench/examples/ridiculous_argument.rs @@ -18,17 +18,17 @@ fn dash_ends_opts() -> bool { #[inline(never)] fn long_flag() -> Option<(&'static str, Option<&'static str>)> { - black_box("--long-flag-with-no-value").parse_long_opt() + black_box("--long-flag-with-no-value").parse_long_opt().unwrap() } #[inline(never)] fn long_flag_blank_value() -> Option<(&'static str, Option<&'static str>)> { - black_box("--long-flag-with-blank-value=").parse_long_opt() + black_box("--long-flag-with-blank-value=").parse_long_opt().unwrap() } #[inline(never)] fn long_flag_value() -> Option<(&'static str, Option<&'static str>)> { - black_box("--long-flag-with-long-value=this-is-a-pretty-longish-value").parse_long_opt() + black_box("--long-flag-with-long-value=this-is-a-pretty-longish-value").parse_long_opt().unwrap() } #[inline(never)] @@ -38,7 +38,7 @@ fn short_flag_cluster() -> Option<&'static str> { #[inline(never)] fn short_flag_cluster_opt() -> (char, Option<&'static str>) { - black_box("verylongshortflagcluster").consume_short_opt() + black_box("verylongshortflagcluster").consume_short_opt().unwrap() } #[inline(never)] @@ -62,18 +62,18 @@ fn bytes_dash_ends_opts() -> bool { } #[inline(never)] -fn bytes_long_flag() -> Option<(&'static [u8], Option<&'static [u8]>)> { - black_box(b"--long-flag-with-no-value").parse_long_opt() +fn bytes_long_flag() -> Option<(&'static str, Option<&'static [u8]>)> { + black_box(b"--long-flag-with-no-value").parse_long_opt().unwrap() } #[inline(never)] -fn bytes_long_flag_blank_value() -> Option<(&'static [u8], Option<&'static [u8]>)> { - black_box(b"--long-flag-with-blank-value=").parse_long_opt() +fn bytes_long_flag_blank_value() -> Option<(&'static str, Option<&'static [u8]>)> { + black_box(b"--long-flag-with-blank-value=").parse_long_opt().unwrap() } #[inline(never)] -fn bytes_long_flag_value() -> Option<(&'static [u8], Option<&'static [u8]>)> { - black_box(b"--long-flag-with-long-value=this-is-a-pretty-longish-value").parse_long_opt() +fn bytes_long_flag_value() -> Option<(&'static str, Option<&'static [u8]>)> { + black_box(b"--long-flag-with-long-value=this-is-a-pretty-longish-value").parse_long_opt().unwrap() } #[inline(never)] @@ -82,8 +82,8 @@ fn bytes_short_flag_cluster() -> Option<&'static [u8]> { } #[inline(never)] -fn bytes_short_flag_cluster_opt() -> (u8, Option<&'static [u8]>) { - black_box(b"verylongshortflagcluster").consume_short_opt() +fn bytes_short_flag_cluster_opt() -> (char, Option<&'static [u8]>) { + black_box(b"verylongshortflagcluster").consume_short_opt().unwrap() } #[inline(never)] diff --git a/bench/examples/vs.rs b/bench/examples/vs.rs index 561e794..cb189fd 100644 --- a/bench/examples/vs.rs +++ b/bench/examples/vs.rs @@ -85,18 +85,18 @@ fn getargs5b<'arg, I: Iterator>(iter: I) -> Settings<&'arg [u while let Some(opt) = opts.next_opt().unwrap() { match opt { - Opt::Short(b'1') => settings.short_present1 = true, - Opt::Short(b'2') => settings.short_present2 = true, - Opt::Short(b'3') => settings.short_present3 = true, - Opt::Long(b"present1") => settings.long_present1 = true, - Opt::Long(b"present2") => settings.long_present2 = true, - Opt::Long(b"present3") => settings.long_present3 = true, - Opt::Short(b'4') => settings.short_value1 = Some(opts.value().unwrap()), - Opt::Short(b'5') => settings.short_value2 = Some(opts.value().unwrap()), - Opt::Short(b'6') => settings.short_value3 = Some(opts.value().unwrap()), - Opt::Long(b"val1") => settings.long_value1 = Some(opts.value().unwrap()), - Opt::Long(b"val2") => settings.long_value2 = Some(opts.value().unwrap()), - Opt::Long(b"val3") => settings.long_value3 = Some(opts.value().unwrap()), + Opt::Short('1') => settings.short_present1 = true, + Opt::Short('2') => settings.short_present2 = true, + Opt::Short('3') => settings.short_present3 = true, + Opt::Long("present1") => settings.long_present1 = true, + Opt::Long("present2") => settings.long_present2 = true, + Opt::Long("present3") => settings.long_present3 = true, + Opt::Short('4') => settings.short_value1 = Some(opts.value().unwrap()), + Opt::Short('5') => settings.short_value2 = Some(opts.value().unwrap()), + Opt::Short('6') => settings.short_value3 = Some(opts.value().unwrap()), + Opt::Long("val1") => settings.long_value1 = Some(opts.value().unwrap()), + Opt::Long("val2") => settings.long_value2 = Some(opts.value().unwrap()), + Opt::Long("val3") => settings.long_value3 = Some(opts.value().unwrap()), _ => {} } } diff --git a/bench/src/evolution.rs b/bench/src/evolution.rs index 5011803..0425768 100644 --- a/bench/src/evolution.rs +++ b/bench/src/evolution.rs @@ -142,18 +142,18 @@ fn getargsLb<'arg, I: Iterator>(iter: I) -> Settings<&'arg [u while let Some(opt) = opts.next_opt().unwrap() { match opt { - Opt::Short(b'1') => settings.short_present1 = true, - Opt::Short(b'2') => settings.short_present2 = true, - Opt::Short(b'3') => settings.short_present3 = true, - Opt::Long(b"present1") => settings.long_present1 = true, - Opt::Long(b"present2") => settings.long_present2 = true, - Opt::Long(b"present3") => settings.long_present3 = true, - Opt::Short(b'4') => settings.short_value1 = Some(opts.value().unwrap()), - Opt::Short(b'5') => settings.short_value2 = Some(opts.value().unwrap()), - Opt::Short(b'6') => settings.short_value3 = Some(opts.value().unwrap()), - Opt::Long(b"val1") => settings.long_value1 = Some(opts.value().unwrap()), - Opt::Long(b"val2") => settings.long_value2 = Some(opts.value().unwrap()), - Opt::Long(b"val3") => settings.long_value3 = Some(opts.value().unwrap()), + Opt::Short('1') => settings.short_present1 = true, + Opt::Short('2') => settings.short_present2 = true, + Opt::Short('3') => settings.short_present3 = true, + Opt::Long("present1") => settings.long_present1 = true, + Opt::Long("present2") => settings.long_present2 = true, + Opt::Long("present3") => settings.long_present3 = true, + Opt::Short('4') => settings.short_value1 = Some(opts.value().unwrap()), + Opt::Short('5') => settings.short_value2 = Some(opts.value().unwrap()), + Opt::Short('6') => settings.short_value3 = Some(opts.value().unwrap()), + Opt::Long("val1") => settings.long_value1 = Some(opts.value().unwrap()), + Opt::Long("val2") => settings.long_value2 = Some(opts.value().unwrap()), + Opt::Long("val3") => settings.long_value3 = Some(opts.value().unwrap()), _ => {} } } diff --git a/examples/complete.rs b/examples/complete.rs index 6fb8c38..b18d2f2 100644 --- a/examples/complete.rs +++ b/examples/complete.rs @@ -9,17 +9,17 @@ enum UsageError<'a> { Help, /// Error from getargs #[error("{0}")] - Getargs(getargs::Error<&'a str>), + Getargs(getargs::Error<'a, &'a str>), #[error("unknown option {0}")] - UnknownOption(Opt<&'a str>), + UnknownOption(Opt<'a>), #[error("unknown subcommand {0:?}")] UnknownSubcommand(&'a str), #[error("missing subcommand")] MissingSubcommand, } -impl<'a> From> for UsageError<'a> { - fn from(e: Error<&'a str>) -> Self { +impl<'a> From> for UsageError<'a> { + fn from(e: Error<'a, &'a str>) -> Self { UsageError::Getargs(e) } } @@ -46,7 +46,7 @@ enum Subcommand<'a> { /// Parse all the arguments fn parse_args<'a, I: Iterator>( - opts: &mut Options<&'a str, I>, + opts: &mut Options<'a, &'a str, I>, ) -> Result, UsageError<'a>> { let mut verbosity = 0; @@ -76,7 +76,7 @@ fn parse_args<'a, I: Iterator>( /// Parse the arguments for the 'create' subcommand fn parse_create_args<'a, I: Iterator>( - opts: &mut Options<&'a str, I>, + opts: &mut Options<'a, &'a str, I>, ) -> Result, UsageError<'a>> { let mut output = None; while let Some(opt) = opts.next_opt()? { @@ -92,7 +92,7 @@ fn parse_create_args<'a, I: Iterator>( /// Parse the arguments for the 'delete' subcommand fn parse_delete_args<'a, I: Iterator>( - opts: &mut Options<&'a str, I>, + opts: &mut Options<'a, &'a str, I>, ) -> Result, UsageError<'a>> { let mut backup = None; while let Some(opt) = opts.next_opt()? { diff --git a/examples/os_str.rs b/examples/os_str.rs index e61efe1..77f307d 100644 --- a/examples/os_str.rs +++ b/examples/os_str.rs @@ -11,7 +11,7 @@ fn main() { while let Some(arg) = opts.next_arg().expect("usage error") { match arg { - Arg::Short(b'f') | Arg::Long(b"file") => { + Arg::Short('f') | Arg::Long("file") => { let f = OsStr::from_bytes(opts.value().expect("usage error")); println!("file option: {f:?}"); } diff --git a/examples/subcommands.rs b/examples/subcommands.rs index 16a81dc..56a10d0 100644 --- a/examples/subcommands.rs +++ b/examples/subcommands.rs @@ -9,8 +9,8 @@ fn main() { } fn parse<'arg, I: Iterator>( - opts: &mut Options<&'arg str, I>, -) -> Result<&'arg str, ()> { + opts: &mut Options<'arg, &'arg str, I>, +) -> Result<'arg, &'arg str, ()> { while let Some(opt) = opts.next_opt()? { println!("option for base command: {opt}"); } @@ -27,8 +27,8 @@ fn parse<'arg, I: Iterator>( } fn parse_run<'arg, I: Iterator>( - opts: &mut Options<&'arg str, I>, -) -> Result<&'arg str, ()> { + opts: &mut Options<'arg, &'arg str, I>, +) -> Result<'arg, &'arg str, ()> { while let Some(opt) = opts.next_opt()? { match opt { Opt::Short('r') | Opt::Long("release") => println!("release mode"), @@ -42,8 +42,8 @@ fn parse_run<'arg, I: Iterator>( } fn parse_test<'arg, I: Iterator>( - opts: &mut Options<&'arg str, I>, -) -> Result<&'arg str, ()> { + opts: &mut Options<'arg, &'arg str, I>, +) -> Result<'arg, &'arg str, ()> { while let Some(opt) = opts.next_opt()? { match opt { Opt::Long("test") => { diff --git a/src/arg.rs b/src/arg.rs index acfab07..3cca08e 100644 --- a/src/arg.rs +++ b/src/arg.rs @@ -8,19 +8,19 @@ use core::fmt::{Debug, Display, Formatter}; /// short or long command-line option name (but not value) like [`Opt`], /// or a positional argument. #[derive(Copy, Clone, Eq, PartialEq, Debug)] -pub enum Arg { +pub enum Arg<'str, A: Argument> { /// A short option, like `-f`. Does not include the leading `-`. - Short(A::ShortOpt), + Short(char), /// A long option, like `--file`. Does not include the leading `--`. - Long(A), + Long(&'str str), /// A positional argument, like `foo.txt`. Positional(A), } -impl Arg { +impl<'str, A: Argument> Arg<'str, A> { /// Retrieves an equivalent [`Opt`] represented by this [`Arg`], if /// it is [`Arg::Short`] or [`Arg::Long`], otherwise `None`. - pub fn opt(self) -> Option> { + pub fn opt(self) -> Option> { match self { Self::Short(short) => Some(Opt::Short(short)), Self::Long(long) => Some(Opt::Long(long)), @@ -38,8 +38,8 @@ impl Arg { } } -impl From> for Arg { - fn from(opt: Opt) -> Self { +impl<'str, A: Argument> From> for Arg<'str, A> { + fn from(opt: Opt<'str>) -> Self { match opt { Opt::Short(short) => Self::Short(short), Opt::Long(long) => Self::Long(long), @@ -47,18 +47,18 @@ impl From> for Arg { } } -impl From for Arg { +impl From for Arg<'_, A> { fn from(arg: A) -> Self { Self::Positional(arg) } } -impl + Display> Display for Arg { +impl Display for Arg<'_, A> { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { match self { Self::Short(short) => write!(f, "-{}", short), Self::Long(long) => write!(f, "--{}", long), - Self::Positional(arg) => Display::fmt(arg, f), + Self::Positional(arg) => arg.display().fmt(f), } } } diff --git a/src/error.rs b/src/error.rs index abe7423..c800874 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,6 +1,6 @@ use core::fmt::{Display, Formatter}; -use crate::{Argument, Opt}; +use crate::{traits::ConversionError, Argument, Opt}; /// An argument parsing error. /// @@ -22,13 +22,13 @@ use crate::{Argument, Opt}; /// been. #[non_exhaustive] #[derive(Copy, Clone, Eq, PartialEq, Debug)] -pub enum Error { +pub enum Error<'opt, A: Argument> { /// The option requires a value, but one was not supplied. /// /// This error is returned when a call to /// [`Options::value`][crate::Options::value`] does not find any /// value to provide. - RequiresValue(Opt), + RequiresValue(Opt<'opt>), /// The option does not require a value, but one was supplied. /// @@ -36,21 +36,32 @@ pub enum Error { /// [`Options::next_opt`][crate::Options::next_opt] or /// [`Options::next_arg`][crate::Options::next_arg] is called /// without the value being consumed. - DoesNotRequireValue(Opt), + DoesNotRequireValue(Opt<'opt>), + + /// The option was not a valid sequence of UTF-8 bytes. + ConversionError(ConversionError), } -impl<'arg, S: Display, A: Argument + Display + 'arg> Display for Error { +impl From> for Error<'_, A> { + fn from(value: ConversionError) -> Self { + Self::ConversionError(value) + } +} + +impl Display for Error<'_, A> { fn fmt(&self, f: &mut Formatter) -> core::fmt::Result { match self { Error::RequiresValue(opt) => write!(f, "option '{}' requires a value", opt), Error::DoesNotRequireValue(opt) => { write!(f, "option '{}' does not expect a value", opt) } + + Error::ConversionError(e) => e.fmt(f), } } } #[cfg(feature = "std")] -impl<'arg, S: Display, A: Argument + Display + 'arg> std::error::Error for Error {} +impl std::error::Error for Error<'_, A> {} -pub type Result = core::result::Result>; +pub type Result<'opt, A, T> = core::result::Result>; diff --git a/src/iter.rs b/src/iter.rs index e8e6087..48f5e3c 100644 --- a/src/iter.rs +++ b/src/iter.rs @@ -4,17 +4,17 @@ use crate::{Argument, Options}; /// /// For more information, see [`Options::positionals`]. #[derive(Debug)] -pub struct Positionals<'opts, A: Argument, I: Iterator> { - inner: &'opts mut Options, +pub struct Positionals<'opts, 'opt, A: Argument, I: Iterator> { + inner: &'opts mut Options<'opt, A, I>, } -impl<'opts, A: Argument, I: Iterator> Positionals<'opts, A, I> { - pub(crate) fn new(inner: &'opts mut Options) -> Self { +impl<'opts, 'opt, A: Argument, I: Iterator> Positionals<'opts, 'opt, A, I> { + pub(crate) fn new(inner: &'opts mut Options<'opt, A, I>) -> Self { Self { inner } } } -impl<'opts, A: Argument, I: Iterator> Iterator for Positionals<'opts, A, I> { +impl<'opts, 'opt, A: Argument, I: Iterator> Iterator for Positionals<'opts, 'opt, A, I> { type Item = A; fn next(&mut self) -> Option { diff --git a/src/lib.rs b/src/lib.rs index 0f2616b..247b7a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -136,7 +136,7 @@ //! # _ => panic!("Unknown option: {}", opt) //! # } //! } -//! # Ok::<(), getargs::Error<&'static str>>(()) +//! # Ok::<(), getargs::Error<'static, &'static str>>(()) //! ``` //! //! [`Options`] notably does not implement [`Iterator`], because a `for` @@ -180,7 +180,7 @@ //! // ... //! } //! -//! # Ok::<(), getargs::Error<&'static str>>(()) +//! # Ok::<(), getargs::Error<'static, &'static str>>(()) //! ``` //! //! Here is the `print` example, which shows required and optional @@ -217,7 +217,7 @@ //! } //! } //! -//! # Ok::<(), getargs::Error<&'static str>>(()) +//! # Ok::<(), getargs::Error<'static, &'static str>>(()) //! ``` //! //! [`Arg`] also has some utility methods, like [`Arg::opt`] and @@ -394,17 +394,17 @@ pub use traits::Argument; /// assert_eq!(opts.next_arg(), Ok(None)); /// ``` #[derive(Debug, Clone)] -pub struct Options> { +pub struct Options<'opt, A: Argument, I: Iterator> { /// Iterator over the arguments. iter: I, /// State information. - state: State, + state: State<'opt, A>, /// Parse short option clusters which look like negative numbers as options allow_number_sopts: bool, } #[derive(Copy, Clone, Debug)] -enum State { +enum State<'opt, A: Argument> { /// The starting state. We may not get a value because there is no /// previous option. We may get a positional argument or an /// option. @@ -417,27 +417,27 @@ enum State { /// value for the option or a positional argument. From here, we /// can get the next option, the next value, or the next /// positional argument. - EndOfOption(Opt), + EndOfOption(Opt<'opt>), /// We are in the middle of a cluster of short options. From here, /// we can get the next short option, or we can get the value for /// the last short option. We may not get a positional argument. ShortOptionCluster(A), /// We just consumed a long option with a value attached with `=`, /// e.g. `--execute=expression`. We must get the following value. - LongOptionWithValue(Opt, A), + LongOptionWithValue(Opt<'opt>, A), /// We have received `None` from the iterator and we are refusing to /// advance to be polite. End { ended_opts: bool }, } -impl<'arg, A: Argument + 'arg, I: Iterator> Options { +impl<'opt, A: Argument, I: Iterator> Options<'opt, A, I> { /// Creates a new [`Options`] given an iterator over arguments of /// type [`A`][Argument]. /// /// The argument parser only lives as long as the iterator, but /// returns arguments with the same lifetime as whatever the /// iterator yields. - pub fn new(iter: I) -> Options { + pub fn new(iter: I) -> Options<'opt, A, I> { Options { iter, state: State::Start { ended_opts: false }, @@ -467,7 +467,10 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { /// /// If your application accepts positional arguments in between /// flags, you can use [`Options::next_arg`] instead of `next_opt`. - pub fn next_opt(&'_ mut self) -> Result>> { + pub fn next_opt(&mut self) -> Result<'opt, A, Option>> + where + A: 'opt, + { match self.state { State::Start { .. } | State::EndOfOption(_) => { let next = self.iter.next(); @@ -482,7 +485,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { if arg.ends_opts() { self.state = State::Start { ended_opts: true }; Ok(None) - } else if let Some((name, value)) = arg.parse_long_opt() { + } else if let Some((name, value)) = arg.parse_long_opt()? { let opt = Opt::Long(name); if let Some(value) = value { @@ -493,7 +496,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { Ok(Some(opt)) } else if let Some(cluster) = arg.parse_short_cluster(self.allow_number_sopts) { - let (opt, rest) = cluster.consume_short_opt(); + let (opt, rest) = cluster.consume_short_opt()?; let opt = Opt::Short(opt); if let Some(rest) = rest { @@ -510,7 +513,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { } State::ShortOptionCluster(rest) => { - let (opt, rest) = rest.consume_short_opt(); + let (opt, rest) = rest.consume_short_opt()?; let opt = Opt::Short(opt); if let Some(rest) = rest { @@ -570,7 +573,10 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { /// # /// # assert_eq!(opts.is_empty(), true); /// ``` - pub fn next_arg(&mut self) -> Result>> { + pub fn next_arg(&mut self) -> Result<'opt, A, Option>> + where + A: 'opt, + { if !self.opts_ended() { if let Some(opt) = self.next_opt()? { return Ok(Some(opt.into())); @@ -608,7 +614,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { /// assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('c')))); /// assert_eq!(opts.value(), Ok("see")); /// ``` - pub fn value(&mut self) -> Result { + pub fn value(&mut self) -> Result<'opt, A, A> { match self.state { State::Start { .. } | State::Positional(_) | State::End { .. } => { panic!("called Options::value() with no previous option") @@ -775,7 +781,7 @@ impl<'arg, A: Argument + 'arg, I: Iterator> Options { /// assert_eq!(args.next(), Some("two")); /// assert_eq!(args.next(), None); /// ``` - pub fn positionals(&mut self) -> Positionals { + pub fn positionals(&mut self) -> Positionals<'_, 'opt, A, I> { Positionals::new(self) } diff --git a/src/opt.rs b/src/opt.rs index 101c87e..1aab264 100644 --- a/src/opt.rs +++ b/src/opt.rs @@ -8,17 +8,17 @@ use crate::{Arg, Argument}; /// [`Options::next_opt`][crate::Options::next_opt] and represents a /// short or long command-line option name (but not value). #[derive(Copy, Clone, Eq, PartialEq, Debug, Hash)] -pub enum Opt { +pub enum Opt<'str> { /// A short option, like `-f`. Does not include the leading `-`. - Short(A::ShortOpt), + Short(char), /// A long option, like `--file`. Does not include the leading `--`. - Long(A), + Long(&'str str), } -impl TryFrom> for Opt { +impl<'str, A: Argument> TryFrom> for Opt<'str> { type Error = (); - fn try_from(value: Arg) -> Result { + fn try_from(value: Arg<'str, A>) -> Result { match value { Arg::Short(short) => Ok(Self::Short(short)), Arg::Long(long) => Ok(Self::Long(long)), @@ -27,7 +27,7 @@ impl TryFrom> for Opt { } } -impl + Display> Display for Opt { +impl Display for Opt<'_> { fn fmt(&self, f: &mut Formatter) -> core::fmt::Result { match self { Opt::Short(c) => write!(f, "-{}", c), diff --git a/src/tests.rs b/src/tests.rs index 77238d4..261cac1 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -412,21 +412,21 @@ fn bytes() { let mut opts = Options::new(args.into_iter()); - assert_eq!(opts.next_opt(), Ok(Some(Opt::Short(b'o')))); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('o')))); assert_eq!(opts.value(), Ok(b"hi".as_slice())); - assert_eq!(opts.next_opt(), Ok(Some(Opt::Long(b"opt".as_slice())))); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Long("opt")))); assert_eq!(opts.value(), Ok(b"HI".as_slice())); - assert_eq!(opts.next_opt(), Ok(Some(Opt::Short(b'o')))); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('o')))); assert_eq!(opts.value(), Ok(b"hi".as_slice())); - assert_eq!(opts.next_opt(), Ok(Some(Opt::Long(b"opt".as_slice())))); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Long("opt")))); assert_eq!(opts.value(), Ok(b"hi".as_slice())); - assert_eq!(opts.next_opt(), Ok(Some(Opt::Long(b"optional".as_slice())))); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Long("optional")))); assert_eq!(opts.value_opt(), None); - assert_eq!(opts.next_opt(), Ok(Some(Opt::Long(b"optional".as_slice())))); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Long("optional")))); assert_eq!(opts.value_opt(), Some(b"value".as_slice())); - assert_eq!(opts.next_opt(), Ok(Some(Opt::Short(b'O')))); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('O')))); assert_eq!(opts.value_opt(), None); - assert_eq!(opts.next_opt(), Ok(Some(Opt::Short(b'O')))); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('O')))); assert_eq!(opts.value_opt(), Some(b"value".as_slice())); assert_eq!(opts.next_opt(), Ok(None)); assert!(opts.opts_ended()); diff --git a/src/traits.rs b/src/traits.rs index f7528ab..f0a7cff 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -23,10 +23,6 @@ use core::fmt::{Debug, Display, Formatter, Write}; /// the inexpensive, zero-copy expectations of argument types. This /// should be a borrow like `&str`, not an owned struct like `String`. pub trait Argument: Copy + Eq + Debug { - /// The short-flag type. For [`&str`], this is [`char`]. For - /// [`&[u8]`][slice], this is `u8`. - type ShortOpt: Copy + Eq + Debug; - /// Returns `true` if this argument signals that no additional /// options should be parsed. If this method returns `true`, then /// [`Options::next_opt`][crate::Options::next_opt] will not attempt @@ -50,12 +46,15 @@ pub trait Argument: Copy + Eq + Debug { /// /// ``` /// # use getargs::Argument; - /// assert_eq!("--flag".parse_long_opt(), Some(("flag", None))); - /// assert_eq!("--flag=value".parse_long_opt(), Some(("flag", Some("value")))); - /// assert_eq!("--flag=".parse_long_opt(), Some(("flag", Some("")))); - /// assert_eq!("-a".parse_long_opt(), None); + /// assert_eq!("--flag".parse_long_opt(), Ok(Some(("flag", None)))); + /// assert_eq!("--flag=value".parse_long_opt(), Ok(Some(("flag", Some("value"))))); + /// assert_eq!("--flag=".parse_long_opt(), Ok(Some(("flag", Some(""))))); + /// assert_eq!("-a".parse_long_opt(), Ok(None)); + ///// assert_eq!(b"--\xFF".parse_long_opt(), Err(ConversionError)); /// ``` - fn parse_long_opt(self) -> Option<(Self, Option)>; + fn parse_long_opt<'opt>(self) -> Result)>> + where + Self: 'opt; /// Attempts to parse this argument as a "short option cluster". /// Returns the short option cluster if present. @@ -106,10 +105,10 @@ pub trait Argument: Copy + Eq + Debug { /// /// ``` /// # use getargs::Argument; - /// assert_eq!("abc".consume_short_opt(), ('a', Some("bc"))); - /// assert_eq!("a".consume_short_opt(), ('a', None)); + /// assert_eq!("abc".consume_short_opt(), Ok(('a', Some("bc")))); + /// assert_eq!("a".consume_short_opt(), Ok(('a', None))); /// ``` - fn consume_short_opt(self) -> (Self::ShortOpt, Option); + fn consume_short_opt(self) -> Result)>; /// Consumes the value of a short option from a "short /// option cluster", as defined by @@ -156,25 +155,50 @@ fn is_number(bytes: &[u8]) -> bool { state != State::Separator } -impl Argument for &str { - type ShortOpt = char; +#[derive(Debug, PartialEq, Clone, Eq, Copy)] +pub struct ConversionError { + option: A, +} + +impl ConversionError { + fn new(option: A) -> Self { + Self { option } + } +} + +impl Display for ConversionError { + fn fmt(&self, f: &mut Formatter) -> core::fmt::Result { + write!(f, "invalid UTF-8 within option: {}", self.option.display()) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for ConversionError {} +pub type Result = core::result::Result>; + +impl Argument for &str { #[inline] fn ends_opts(self) -> bool { self == "--" } #[inline] - fn parse_long_opt(self) -> Option<(Self, Option)> { + fn parse_long_opt<'opt>(self) -> Result)>> + where + Self: 'opt, + { // Using iterators is slightly faster in release, but many times // (>400%) as slow in dev - let option = self.strip_prefix("--").filter(|s| !s.is_empty())?; + let Some(option) = self.strip_prefix("--").filter(|s| !s.is_empty()) else { + return Ok(None); + }; if let Some((option, value)) = option.split_once('=') { - Some((option, Some(value))) + Ok(Some((option, Some(value)))) } else { - Some((option, None)) + Ok(Some((option, None))) } } @@ -185,7 +209,7 @@ impl Argument for &str { } #[inline] - fn consume_short_opt(self) -> (Self::ShortOpt, Option) { + fn consume_short_opt(self) -> Result)> { let ch = self .chars() .next() @@ -193,7 +217,7 @@ impl Argument for &str { // using `unsafe` here only improves performance by ~10% and is // not worth it for losing the "we don't use `unsafe`" guarantee - (ch, Some(&self[ch.len_utf8()..]).filter(|s| !s.is_empty())) + Ok((ch, Some(&self[ch.len_utf8()..]).filter(|s| !s.is_empty()))) } #[inline] @@ -226,16 +250,19 @@ impl Display for DisplaySliceU8<'_> { } impl Argument for &[u8] { - type ShortOpt = u8; - #[inline] fn ends_opts(self) -> bool { self == b"--" } #[inline] - fn parse_long_opt(self) -> Option<(Self, Option)> { - let option = self.strip_prefix(b"--").filter(|a| !a.is_empty())?; + fn parse_long_opt<'opt>(self) -> Result)>> + where + Self: 'opt, + { + let Some(option) = self.strip_prefix(b"--").filter(|a| !a.is_empty()) else { + return Ok(None); + }; // This is faster than iterators in dev let name = option.split(|b| *b == b'=').next().unwrap(); @@ -245,7 +272,11 @@ impl Argument for &[u8] { None }; - Some((name, value)) + let Ok(name) = core::str::from_utf8(name) else { + return Err(ConversionError::new(self)); + }; + + Ok(Some((name, value))) } #[inline] @@ -255,12 +286,24 @@ impl Argument for &[u8] { } #[inline] - fn consume_short_opt(self) -> (Self::ShortOpt, Option) { - let (byte, rest) = self - .split_first() - .expect("<&[u8] as getargs::Argument>::consume_short_opt called on an empty slice"); - - (*byte, Some(rest).filter(|s| !s.is_empty())) + fn consume_short_opt(self) -> Result)> { + if self[0].is_ascii() { + Ok((self[0] as char, Some(&self[1..]).filter(|s| !s.is_empty()))) + } else { + let Some(ch) = self + .utf8_chunks() + .next() + .expect("<&[u8] as getargs::Argument>::consume_short_opt called on an empty string") + .valid() + .chars() + .next() + else { + return Err(ConversionError::new(self)); + }; + + let rest = &self[ch.len_utf8()..]; + Ok((ch, Some(rest).filter(|s| !s.is_empty()))) + } } #[inline] From d2bd7f8b109deb8e2082179cc78976b136b440fc Mon Sep 17 00:00:00 2001 From: Tomasz Kramkowski Date: Tue, 1 Oct 2024 23:58:39 +0100 Subject: [PATCH 8/8] &OsStr support Implement `&OsStr as Argument` on top of `&[u8] as Argument`. The unsafe code to convert back from the sliced `&[u8]` keeps to the safety invariants defined for the `OsStr::from_encoded_bytes_unchecked` function. --- README.md | 6 ++-- bench/examples/vs.rs | 36 +++++++++++++++++++ bench/src/evolution.rs | 79 ++++++++++++++++++++++++++++++++++++++++++ examples/no_alloc.rs | 9 ++--- examples/os_str.rs | 22 ++++-------- src/lib.rs | 4 +-- src/tests.rs | 47 +++++++++++++++++++++++++ src/traits.rs | 63 +++++++++++++++++++++++++++++++++ 8 files changed, 240 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 776a485..47b7c9f 100644 --- a/README.md +++ b/README.md @@ -28,12 +28,12 @@ you do with it. * Zero cost * Zero copy -* Zero unsafe code +* Zero unsafe code except for `&OsStr` * Zero dependencies * Zero allocation * Simple to use yet versatile -* `#![no_std]`-compatible -* Compatible with `&str` and `&[u8]` +* `#![no_std]`-compatible except for `&OsStr` +* Compatible with `&str`, `&[u8]` and `&OsStr` ## Performance diff --git a/bench/examples/vs.rs b/bench/examples/vs.rs index cb189fd..9caaeac 100644 --- a/bench/examples/vs.rs +++ b/bench/examples/vs.rs @@ -1,5 +1,6 @@ use bench::{ARGS, ARGS_BYTES}; use getargs::Argument; +use std::ffi::OsStr; use std::hint::black_box; use std::time::Instant; @@ -104,6 +105,34 @@ fn getargs5b<'arg, I: Iterator>(iter: I) -> Settings<&'arg [u settings } +#[inline(never)] +fn getargs5o<'arg, I: Iterator>(iter: I) -> Settings<&'arg OsStr> { + use getargs::{Opt, Options}; + + let mut settings = Settings::default(); + let mut opts = Options::new(iter); + + while let Some(opt) = opts.next_opt().unwrap() { + match opt { + Opt::Short('1') => settings.short_present1 = true, + Opt::Short('2') => settings.short_present2 = true, + Opt::Short('3') => settings.short_present3 = true, + Opt::Long("present1") => settings.long_present1 = true, + Opt::Long("present2") => settings.long_present2 = true, + Opt::Long("present3") => settings.long_present3 = true, + Opt::Short('4') => settings.short_value1 = Some(opts.value().unwrap()), + Opt::Short('5') => settings.short_value2 = Some(opts.value().unwrap()), + Opt::Short('6') => settings.short_value3 = Some(opts.value().unwrap()), + Opt::Long("val1") => settings.long_value1 = Some(opts.value().unwrap()), + Opt::Long("val2") => settings.long_value2 = Some(opts.value().unwrap()), + Opt::Long("val3") => settings.long_value3 = Some(opts.value().unwrap()), + _ => {} + } + } + + settings +} + fn main() { const ITERATIONS: usize = 10_000_000; @@ -127,7 +156,14 @@ fn main() { let d = Instant::now(); + for _ in 0..ITERATIONS { + black_box(getargs5o(ARGS.iter().copied().map(AsRef::as_ref))); + } + + let e = Instant::now(); + eprintln!("getargs4: {}ns", (b - a).as_nanos() / ITERATIONS as u128); eprintln!("getargs5: {}ns", (c - b).as_nanos() / ITERATIONS as u128); eprintln!("getargs5b: {}ns", (d - c).as_nanos() / ITERATIONS as u128); + eprintln!("getargs5o: {}ns", (e - d).as_nanos() / ITERATIONS as u128); } diff --git a/bench/src/evolution.rs b/bench/src/evolution.rs index 0425768..37e03c6 100644 --- a/bench/src/evolution.rs +++ b/bench/src/evolution.rs @@ -1,5 +1,7 @@ #![allow(non_snake_case)] +use std::ffi::OsStr; + use crate::{ARGS, ARGS_BYTES}; use getargs::Argument; use test::Bencher; @@ -161,6 +163,34 @@ fn getargsLb<'arg, I: Iterator>(iter: I) -> Settings<&'arg [u settings } +#[inline(always)] +fn getargsLo<'arg, I: Iterator>(iter: I) -> Settings<&'arg OsStr> { + use getargs::{Opt, Options}; + + let mut settings = Settings::default(); + let mut opts = Options::new(iter); + + while let Some(opt) = opts.next_opt().unwrap() { + match opt { + Opt::Short('1') => settings.short_present1 = true, + Opt::Short('2') => settings.short_present2 = true, + Opt::Short('3') => settings.short_present3 = true, + Opt::Long("present1") => settings.long_present1 = true, + Opt::Long("present2") => settings.long_present2 = true, + Opt::Long("present3") => settings.long_present3 = true, + Opt::Short('4') => settings.short_value1 = Some(opts.value().unwrap()), + Opt::Short('5') => settings.short_value2 = Some(opts.value().unwrap()), + Opt::Short('6') => settings.short_value3 = Some(opts.value().unwrap()), + Opt::Long("val1") => settings.long_value1 = Some(opts.value().unwrap()), + Opt::Long("val2") => settings.long_value2 = Some(opts.value().unwrap()), + Opt::Long("val3") => settings.long_value3 = Some(opts.value().unwrap()), + _ => {} + } + } + + settings +} + #[bench] #[inline(never)] fn getargs4_varied_small(bencher: &mut Bencher) { @@ -191,6 +221,13 @@ fn getargsLb_varied_small(bencher: &mut Bencher) { bencher.iter(|| getargsLb(ARGS_BYTES.iter().copied())); } +#[bench] +#[inline(never)] +fn getargsLo_varied_small(bencher: &mut Bencher) { + let args_os: Box<[&OsStr]> = ARGS.iter().copied().map(AsRef::as_ref).collect(); + bencher.iter(|| getargsLo(args_os.iter().copied())); +} + pub const ARGS_LONG: [&str; 1000] = ["--dsfigadsjfdgsfjkasbfjksdfabsdbfdaf"; 1000]; pub const ARGS_LONG_BYTES: [&[u8]; 1000] = [b"--dsfigadsjfdgsfjkasbfjksdfabsdbfdaf"; 1000]; @@ -224,6 +261,13 @@ fn getargsLb_long(bencher: &mut Bencher) { bencher.iter(|| getargsLb(ARGS_LONG_BYTES.iter().copied())); } +#[bench] +#[inline(never)] +fn getargsLo_long(bencher: &mut Bencher) { + let args_os: Vec<&OsStr> = ARGS_LONG.iter().copied().map(AsRef::as_ref).collect(); + bencher.iter(|| getargsLo(args_os.iter().copied())); +} + pub const ARGS_SHORT_CLUSTER: [&str; 1000] = ["-rjryets8kzrlxu7lzvnmsooiac8u9lxluphwrfudxaitfdomtce78grull9cpcvk7lyi07mdoclybtolssg7w7kwei79k"; 1000]; @@ -260,6 +304,13 @@ fn getargsLb_short_cluster(bencher: &mut Bencher) { bencher.iter(|| getargsLb(ARGS_SHORT_CLUSTER_BYTES.iter().copied())); } +#[bench] +#[inline(never)] +fn getargsLo_short_cluster(bencher: &mut Bencher) { + let args_os: Vec<&OsStr> = ARGS_SHORT_CLUSTER.iter().copied().map(AsRef::as_ref).collect(); + bencher.iter(|| getargsLo(args_os.iter().copied())); +} + pub const ARGS_SHORT_EVALUE: [&str; 1000] = ["-rjryets8kzrlxu7lzvnmso4oiac8u9lxluphwrfudxaitfdomtce78grull9cpcvk7lyi07mdoclybtolssg7w7kwei79k"; 1000]; @@ -296,6 +347,13 @@ fn getargsLb_short_evalue(bencher: &mut Bencher) { bencher.iter(|| getargsLb(ARGS_SHORT_EVALUE_BYTES.iter().copied())); } +#[bench] +#[inline(never)] +fn getargsLo_short_evalue(bencher: &mut Bencher) { + let args_os: Vec<&OsStr> = ARGS_SHORT_EVALUE.iter().copied().map(AsRef::as_ref).collect(); + bencher.iter(|| getargsLo(args_os.iter().copied())); +} + pub const ARGS_SHORT_IVALUE: [&str; 1000] = ["-rjryets8kzrlxu7lzvnmsooiac8u9lxluphwrfudxaitfdomtce78grull9cpcvk7lyi07mdoclybtolssg7w7kwei79k4"; 1000]; @@ -332,6 +390,13 @@ fn getargsLb_short_ivalue(bencher: &mut Bencher) { bencher.iter(|| getargsLb(ARGS_SHORT_IVALUE_BYTES.iter().copied())); } +#[bench] +#[inline(never)] +fn getargsLo_short_ivalue(bencher: &mut Bencher) { + let args_os: Vec<&OsStr> = ARGS_SHORT_IVALUE.iter().copied().map(AsRef::as_ref).collect(); + bencher.iter(|| getargsLo(args_os.iter().copied())); +} + pub const ARGS_LONG_EVALUE: [&str; 1000] = ["--val1=rjryets8kzrlxu7lzvnms4ooiac8u9lxluphwrfudxaitfdomtce78grull9cpcvk7lyi07mdoclybtolssg7w7kwei79k"; 1000]; @@ -368,6 +433,13 @@ fn getargsLb_long_evalue(bencher: &mut Bencher) { bencher.iter(|| getargsLb(ARGS_LONG_EVALUE_BYTES.iter().copied())); } +#[bench] +#[inline(never)] +fn getargsLo_long_evalue(bencher: &mut Bencher) { + let args_os: Vec<&OsStr> = ARGS_LONG_EVALUE.iter().copied().map(AsRef::as_ref).collect(); + bencher.iter(|| getargsLo(args_os.iter().copied())); +} + pub const ARGS_LONG_IVALUE: [&str; 1000] = ["--val1"; 1000]; pub const ARGS_LONG_IVALUE_BYTES: [&[u8]; 1000] = [b"--val1"; 1000]; @@ -400,3 +472,10 @@ fn getargsL_long_ivalue(bencher: &mut Bencher) { fn getargsLb_long_ivalue(bencher: &mut Bencher) { bencher.iter(|| getargsLb(ARGS_LONG_IVALUE_BYTES.iter().copied())); } + +#[bench] +#[inline(never)] +fn getargsLo_long_ivalue(bencher: &mut Bencher) { + let args_os: Vec<&OsStr> = ARGS_LONG_IVALUE.iter().copied().map(AsRef::as_ref).collect(); + bencher.iter(|| getargsLo(args_os.iter().copied())); +} diff --git a/examples/no_alloc.rs b/examples/no_alloc.rs index 834fe41..a682829 100644 --- a/examples/no_alloc.rs +++ b/examples/no_alloc.rs @@ -5,13 +5,10 @@ //! Additionally, all strings and errors are annotated with the correct lifetimes, so that the //! lifetime of the iterator itself does not matter so much anymore. -use getargs::{Opt, Options}; +use getargs::{Argument, Opt, Options}; fn main() { - let args = argv::iter().skip(1).map(|os| { - os.to_str() - .expect("argument couldn't be converted to UTF-8") - }); + let args = argv::iter().skip(1); let mut opts = Options::new(args); @@ -24,6 +21,6 @@ fn main() { } for positional in opts.positionals() { - eprintln!("positional argument: {}", positional); + eprintln!("positional argument: {}", Argument::display(positional)); } } diff --git a/examples/os_str.rs b/examples/os_str.rs index 77f307d..a7fa0ff 100644 --- a/examples/os_str.rs +++ b/examples/os_str.rs @@ -1,30 +1,22 @@ -use getargs::{Arg, Options}; -use std::ffi::OsStr; +use getargs::{Arg, Argument, Options}; +use std::ffi::OsString; +use std::path::PathBuf; -#[cfg(unix)] fn main() { - use std::os::unix::ffi::OsStrExt; - let args: Vec<_> = std::env::args_os().skip(1).collect(); - let mut opts = Options::new(args.iter().map(|s| s.as_bytes())); + let mut opts = Options::new(args.iter().map(OsString::as_os_str)); while let Some(arg) = opts.next_arg().expect("usage error") { match arg { Arg::Short('f') | Arg::Long("file") => { - let f = OsStr::from_bytes(opts.value().expect("usage error")); + let f = PathBuf::from(opts.value().expect("usage error")); println!("file option: {f:?}"); } Arg::Positional(pos) => { - let pos = OsStr::from_bytes(pos); - println!("positional: {pos:?}"); + println!("positional: {}", Argument::display(pos)); } - _ => println!("other: {arg:?}"), + _ => println!("other: {}", arg), } } } - -#[cfg(not(unix))] -fn main() { - eprintln!("Only supported on Unix because UTF-16 is hard, sorry :("); -} diff --git a/src/lib.rs b/src/lib.rs index 247b7a2..8eeb11d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,12 +28,12 @@ //! //! * Zero cost //! * Zero copy -//! * Zero unsafe code +//! * Zero unsafe code except for `&OsStr` //! * Zero dependencies //! * Zero allocation //! * Simple to use yet versatile //! * `#![no_std]`-compatible -//! * Compatible with `&str` and `&[u8]` +//! * Compatible with `&str`, `&[u8]` and `&OsStr` //! //! ## Performance //! diff --git a/src/tests.rs b/src/tests.rs index 261cac1..274ce4a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,3 +1,5 @@ +use std::ffi::OsStr; + use super::*; #[test] @@ -574,3 +576,48 @@ fn repeating_iterator() { assert_eq!(opts.next_positional(), None); assert!(opts.is_empty()); } + +#[test] +fn os_str() { + let args = [ + "-ohi", + "--opt=HI", + "-o", + "hi", + "--opt", + "hi", + "--optional", + "--optional=value", + "-O", + "-Ovalue", + "--", + "one", + "two", + ]; + + let mut opts = Options::<'_, &OsStr, _>::new(args.into_iter().map(AsRef::as_ref)); + + assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('o')))); + assert_eq!(opts.value(), Ok("hi".as_ref())); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Long("opt")))); + assert_eq!(opts.value(), Ok("HI".as_ref())); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('o')))); + assert_eq!(opts.value(), Ok("hi".as_ref())); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Long("opt")))); + assert_eq!(opts.value(), Ok("hi".as_ref())); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Long("optional")))); + assert_eq!(opts.value_opt(), None); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Long("optional")))); + assert_eq!(opts.value_opt(), Some("value".as_ref())); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('O')))); + assert_eq!(opts.value_opt(), None); + assert_eq!(opts.next_opt(), Ok(Some(Opt::Short('O')))); + assert_eq!(opts.value_opt(), Some("value".as_ref())); + assert_eq!(opts.next_opt(), Ok(None)); + assert!(opts.opts_ended()); + assert_eq!(opts.next_positional(), Some("one".as_ref())); + assert_eq!(opts.next_positional(), Some("two".as_ref())); + assert_eq!(opts.next_positional(), None); + assert!(opts.opts_ended()); + assert!(opts.is_empty()); +} diff --git a/src/traits.rs b/src/traits.rs index f0a7cff..2199fb0 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -1,4 +1,6 @@ use core::fmt::{Debug, Display, Formatter, Write}; +#[cfg(feature = "std")] +use std::ffi::OsStr; /// The argument trait for types that can be parsed by /// [`Options`][crate::Options]. @@ -317,6 +319,67 @@ impl Argument for &[u8] { } } +#[cfg(feature = "std")] +impl Argument for &OsStr { + #[inline] + fn ends_opts(self) -> bool { + self.as_encoded_bytes().ends_opts() + } + + #[inline] + fn parse_long_opt<'opt>(self) -> Result)>> + where + Self: 'opt, + { + self.as_encoded_bytes() + .parse_long_opt() + .map(|o| { + o.map(|t| { + ( + t.0, + t.1.map(|s| unsafe { OsStr::from_encoded_bytes_unchecked(s) }), + ) + }) + }) + .map_err(|e| { + ConversionError::new(unsafe { OsStr::from_encoded_bytes_unchecked(e.option) }) + }) + } + + #[inline] + fn parse_short_cluster(self, allow_number: bool) -> Option { + self.as_encoded_bytes() + .parse_short_cluster(allow_number) + .map(|s| unsafe { OsStr::from_encoded_bytes_unchecked(s) }) + } + + #[inline] + fn consume_short_opt(self) -> Result)> { + self.as_encoded_bytes() + .consume_short_opt() + .map(|t| { + ( + t.0, + t.1.map(|s| unsafe { OsStr::from_encoded_bytes_unchecked(s) }), + ) + }) + .map_err(|e| { + ConversionError::new(unsafe { OsStr::from_encoded_bytes_unchecked(e.option) }) + }) + } + + #[inline] + fn consume_short_val(self) -> Self { + self + } + + fn display(self) -> impl Display { + DisplaySliceU8 { + slice: self.as_encoded_bytes(), + } + } +} + #[cfg(test)] mod tests { use super::is_number;