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

Positional arguments redo #70

Closed
tertsdiepraam opened this issue Dec 13, 2023 · 3 comments · Fixed by #72
Closed

Positional arguments redo #70

tertsdiepraam opened this issue Dec 13, 2023 · 3 comments · Fixed by #72

Comments

@tertsdiepraam
Copy link
Member

tertsdiepraam commented Dec 13, 2023

I'm not happy with positional arguments at the moment. They do not fit nicely with the rest of the library and have a weird API. We need to reassess how they should work.

The problems are:

  1. The positional arguments do not really fit in an enum.
  2. The distribution of positional arguments cannot be dependent on options (see for instance what cp requires).
  3. The ranges are unintuitive and have no "obvious" default value.
@tertsdiepraam
Copy link
Member Author

tertsdiepraam commented Dec 14, 2023

I'll post several comments exploring some designs.

The simplest alternative is to change the implementation a bit by removing the ranges and instead encoding the number of arguments in the specification:

#[derive(Arguments)]
enum Arg {
    #[arg("VAL")]      // 1
    Foo,
    #[arg("[VAL]")]    // 0 or 1
    Bar,
    #[arg("VAL...")]   // 1 or more
    Baz,
    #[arg("[VAL]...")] // 0 or more
    Qux,
}

This brings it more in line with the option arguments in how it is defined. However this only really solves the third problem.

@tertsdiepraam
Copy link
Member Author

A second option is to take the positional arguments out of the enum and pass them as Vec to the Settings:

#[derive(Arguments)]
#[arguments(positional = "VAL1 [VAL2]...")]
enum Arg {}

struct Settings {
    a: PathBuf,
    b: Vec<PathBuf>,
}

impl Options<Arg> for Settings {
    fn apply(&mut self, arg: Arg) {
        ...
    }
    
    fn apply_positional(&mut sef, args: Vec<PathBuf>) {
        ...
    }
}

This "solves" all the problems, but is also verbose. It also makes error handling for positional arguments more difficult.

@tertsdiepraam
Copy link
Member Author

tertsdiepraam commented Dec 14, 2023

A third option is to not try to really integrate the positional arguments at all:

#[derive(Arguments)]
enum Arg { ... }

#[derive(Default)]
struct Settings { ... }

impl Options<Arg> for Settings { ... }

fn main() {
    // `operands` is a `Vec<OsString>` with the positional arguments:
    let (settings, operands) = Settings::default().parse();
}

Instead of just having a Vec, it could also be a wrapper type around a Vec that could provide better error messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant