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

feat: add require no space to argument short version #5489

Closed
wants to merge 2 commits into from

Conversation

my4ng
Copy link

@my4ng my4ng commented May 9, 2024

Fix #3030.

This PR enables restricting the short option to the -oval syntax with the require_no_space argument setting, and changes require_equals to only affect the long option --option=val (BREAKING CHANGE). This will allow compliance with the POSIX and GNU conventions as mentioned in the issue by setting both require_no_space and require_equals to true:

In clap terms, these are saying we should only allow

  • --long
  • --long=value
  • -s
  • -svalue

Here is an example help message that demonstrates this:

Usage: myapp [OPTIONS]

Options:
  -f[<foo>], --foo[=<foo>]  This is foo
  -h, --help                Print help

A specific real-world application that has motivated this is uutils/coreutils which aims to mirror the GNU version as closely as possible. However, their date -I/--iso-8601 implementation, for example, accepts values that are not considered valid (e.g. date -I seconds and date -I=seconds), and there is no possible workaround.

The changes made to require_equals may break current applications that take a positional argument after an optional option-argument, e.g.

Command::new("prog")
    .arg(
        Arg::new("cfg")
            .action(ArgAction::Set)
            .require_equals(true)
            .num_args(0..)
            .short('c'),
    )
    .arg(Arg::new("cmd"))
    .try_get_matches_from(vec!["prog", "-c", "cmd"]);

Currently cmd would be treated as the second argument, but after the PR where require_equals no longer works on short option, it will become the associated value to the first.

Migration to the new major version (if this is merged) should complement require_equals with require_no_space and replace -o=val usage with -oval, otherwise the associated value would become =val as = is parsed as part of the value.

my4ng added 2 commits May 9, 2024 14:42
BREAKING CHANGE: `require_equals` no longer affects the short option
thus this may break some arguments. For example, if an argument has
`require_equals` and `min_vals == 0`, `val` in `-o val` is now parsed as
the argument value rather than a separate positional argument.

Introduce `require_no_space` which requires that the short option has no
space between it and the associated value.

Limit `require_equals` to only the long option.
Remove and modify some `require_equals` tests that are no longer valid.
@epage
Copy link
Member

epage commented May 9, 2024

Thank you for your interest in this and the work that you put into this! However, we are not ready to make a breaking change at this time and we have not fully decided on a direction for this. I would instead recommend you propose your idea in the thread of #3030 and see where that goes. As this is too early for this PR and its unclear if this is the PR that we want, I'm going to go ahead and close this for now.

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 this pull request may close these issues.

Optional option-argument parsing is incorrect
2 participants