Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Options::apply fallible #113

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,12 @@ struct Settings {
// To implement `Options`, we only need to provide the `apply` method.
// The `parse` method will be automatically generated.
impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Caps => self.caps = true,
Arg::ExclamationMarks(n) => self.exclamation_marks += n,
}
Ok(())
}
}

Expand Down
16 changes: 13 additions & 3 deletions docs/design/problems_with_clap.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ uutils and when we opened as issue for it, it was discarded. This makes sense
from `clap`'s perspective, but it shows that the priorities between `clap` and
uutils diverge.

## Problem 6: It's stringly typed
## Problem 7: It's stringly typed

`clap`'s arguments are identified by strings. This leads to code like this:

Expand All @@ -135,21 +135,31 @@ deal, but a bit annoying.

Of course, we wouldn't have this problem if we were able to use the derive API.

## Problem 7: Reading help string from a file
## Problem 8: Reading help string from a file

In `uutils` our help strings can get quite long. Therefore, we like to extract
those to an external file. With `clap` this means that we need to do some custom
preprocessing on this file to extract the information for the several pieces of
the help string that `clap` supports.

## Problem 8: No markdown support
## Problem 9: No markdown support

Granted, this is not really a problem, but more of a nice-to-have. We have
online documentation for the utils, based on the help strings and these are
rendered from markdown. Ideally, our argument parser supports markdown too, so
that we can have nicely rendered help strings which have (roughly) the same
appearance in the terminal and online.

## Problem 10: No position-dependent argument-error prioritization

This is the question of which error to print if both `-A` and `-B` are given,
and both are individually an error somehow. In case of the GNU tools, only the
first error is printed, and then the program is aborted.

This also is not really a problem, but since it can be reasonably easily
achieved by simply raising an error during argument application, this enables
matching more closely the exact behavior of the GNU tools.

## Good things about `clap`

Alright, enough problems. Let's praise `clap` a bit, because it's an excellent
Expand Down
17 changes: 11 additions & 6 deletions docs/guide/port.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,11 @@ enum Arg {
struct Settings { a: bool }

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::A => self.a = true,
}
Ok(())
}
}

Expand Down Expand Up @@ -137,10 +138,11 @@ impl Default for Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::A => self.a = false,
}
Ok(())
}
}

Expand Down Expand Up @@ -175,10 +177,11 @@ enum Arg {
struct Settings { a: u8 }

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::A => self.a += 1,
}
Ok(())
}
}

Expand Down Expand Up @@ -215,10 +218,11 @@ enum Arg {
struct Settings { a: OsString }

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::A(s) => self.a = s,
}
Ok(())
}
}

Expand Down Expand Up @@ -255,10 +259,11 @@ enum Arg {
struct Settings { a: Vec<OsString> }

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::A(s) => self.a.push(s),
}
Ok(())
}
}

Expand All @@ -271,4 +276,4 @@ let a = Settings::default().parse(std::env::args_os()).unwrap().0.a;
[Up](super)
[Next](next)

</div>
</div>
20 changes: 13 additions & 7 deletions docs/guide/quick.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,11 @@ struct Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Force => self.force = true,
}
Ok(())
}
}

Expand Down Expand Up @@ -100,11 +101,12 @@ struct Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Force => self.force = true,
Arg::NoForce => self.force = false,
}
Ok(())
}
}

Expand Down Expand Up @@ -160,10 +162,11 @@ enum Arg {
# }
#
# impl Options<Arg> for Settings {
# fn apply(&mut self, arg: Arg) {
# fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
# match arg {
# Arg::Name(name) => self.name = name,
# }
# Ok(())
# }
# }
#
Expand Down Expand Up @@ -197,10 +200,11 @@ enum Arg {
# }
#
# impl Options<Arg> for Settings {
# fn apply(&mut self, arg: Arg) {
# fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
# match arg {
# Arg::Name(name) => self.name = name,
# }
# Ok(())
# }
# }
#
Expand Down Expand Up @@ -234,10 +238,11 @@ enum Arg {
# }
#
# impl Options<Arg> for Settings {
# fn apply(&mut self, arg: Arg) {
# fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
# match arg {
# Arg::Force(b) => self.force = b,
# }
# Ok(())
# }
# }
#
Expand Down Expand Up @@ -269,10 +274,11 @@ enum Arg {
# }
#
# impl Options<Arg> for Settings {
# fn apply(&mut self, arg: Arg) {
# fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
# match arg {
# Arg::Sort(s) => self.sort = s,
# }
# Ok(())
# }
# }
#
Expand All @@ -287,4 +293,4 @@ enum Arg {
[Up](super)
[Next](next)

</div>
</div>
6 changes: 3 additions & 3 deletions examples/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ enum Arg {
// Completion is derived from the `Number` type, through the `Value` trait
/// Give it a number!
#[arg("-n N", "--number=N")]
Number(Number),
Number(#[allow(unused)] Number),

// Completion is derived from the `PathBuf` type
/// Give it a path!
#[arg("-p P", "--path=P")]
Path(PathBuf),
Path(#[allow(unused)] PathBuf),
}

struct Settings;

impl Options<Arg> for Settings {
fn apply(&mut self, _arg: Arg) {
fn apply(&mut self, _arg: Arg) -> Result<(), uutils_args::Error> {
panic!("Compile with the 'parse-is-complete' feature!")
}
}
Expand Down
3 changes: 2 additions & 1 deletion examples/deprecated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ struct Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Min(n) => self.n1 = n,
Arg::Plus(n) => self.n2 = n,
}
Ok(())
}
}

Expand Down
3 changes: 2 additions & 1 deletion examples/hello_world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ struct Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Name(n) => self.name = n,
Arg::Count(c) => self.count = c,
Arg::Hidden => {}
}
Ok(())
}
}

Expand Down
3 changes: 2 additions & 1 deletion examples/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ struct Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Color(c) => self.color = c,
}
Ok(())
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl<T: Arguments> ArgumentIter<T> {
/// call [`Options::apply`] on the result until the arguments are exhausted.
pub trait Options<Arg: Arguments>: Sized {
/// Apply a single argument to the options.
fn apply(&mut self, arg: Arg);
fn apply(&mut self, arg: Arg) -> Result<(), Error>;

/// Parse an iterator of arguments into the options
#[allow(unused_mut)]
Expand All @@ -191,7 +191,7 @@ pub trait Options<Arg: Arguments>: Sized {
{
let mut iter = ArgumentIter::<Arg>::from_args(args);
while let Some(arg) = iter.next_arg()? {
self.apply(arg);
self.apply(arg)?;
}
Ok((self, iter.positional_arguments))
}
Expand Down
3 changes: 3 additions & 0 deletions tests/coreutils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ mod cat;
#[path = "coreutils/cksum.rs"]
mod cksum;

#[path = "coreutils/date.rs"]
mod date;

#[path = "coreutils/dd.rs"]
mod dd;

Expand Down
3 changes: 2 additions & 1 deletion tests/coreutils/b2sum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ struct Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Binary => self.binary = true,
Arg::Check => self.check = true,
Expand All @@ -57,6 +57,7 @@ impl Options<Arg> for Settings {
Arg::Strict => self.strict = true,
Arg::Warn => self.check_output = CheckOutput::Warn,
}
Ok(())
}
}

Expand Down
3 changes: 2 additions & 1 deletion tests/coreutils/base32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,14 @@ impl Default for Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Decode => self.decode = true,
Arg::IgnoreGarbage => self.ignore_garbage = true,
Arg::Wrap(0) => self.wrap = None,
Arg::Wrap(x) => self.wrap = Some(x),
}
Ok(())
}
}

Expand Down
3 changes: 2 additions & 1 deletion tests/coreutils/basename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ struct Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Multiple => self.multiple = true,
Arg::Suffix(s) => {
Expand All @@ -35,6 +35,7 @@ impl Options<Arg> for Settings {
}
Arg::Zero => self.zero = true,
}
Ok(())
}
}

Expand Down
3 changes: 2 additions & 1 deletion tests/coreutils/cat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ struct Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::ShowAll => {
self.show_tabs = true;
Expand All @@ -70,6 +70,7 @@ impl Options<Arg> for Settings {
Arg::NumberNonblank => self.number = NumberingMode::NonEmpty,
Arg::SqueezeBlank => self.squeeze_blank = true,
}
Ok(())
}
}

Expand Down
3 changes: 2 additions & 1 deletion tests/coreutils/cksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ struct Settings {
}

impl Options<Arg> for Settings {
fn apply(&mut self, arg: Arg) {
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
match arg {
Arg::Binary => self.binary = Tristate::True,
Arg::Text => self.binary = Tristate::False,
Expand All @@ -47,6 +47,7 @@ impl Options<Arg> for Settings {
self.tag = Tristate::False;
}
}
Ok(())
}
}

Expand Down
Loading