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 last take precedence over allow_hyphen_values #5039

Open
2 tasks done
mamekoro opened this issue Jul 22, 2023 · 0 comments
Open
2 tasks done

Make last take precedence over allow_hyphen_values #5039

mamekoro opened this issue Jul 22, 2023 · 0 comments
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-enhancement Category: Raise on the bar on expectations S-waiting-on-decision Status: Waiting on a go/no-go before implementing

Comments

@mamekoro
Copy link

mamekoro commented Jul 22, 2023

Please complete the following tasks

Clap Version

4.3.19

Describe your use case

former discussion: #4960

My program needs 2 kinds of command line arguments.
One is the options for the program itself (called opts), and the other is the command and its arguments passed to an external program (called cmdline).

I'd like to distinguish between opts and cmdline by separating them with a double hyphen --. However, it does not work because allow_hyphen_values has higher precedence than last.

Here is an example:

use clap::Parser;

#[derive(Clone, Debug, Parser)]
pub struct Args {
    #[arg(allow_hyphen_values = true)]
    pub opts: Vec<String>,

    #[arg(allow_hyphen_values = true, last = true)]
    pub cmdline: Vec<String>,
}

fn main() {
    let args = Args::parse();
    println!("opts = {:?}, cmdline = {:?}", args.opts, args.cmdline);
}

Run it in Bash like following.
As you can see, last = true is not in effect in the first 2 calls.

alias run='cargo run --quiet --'

run --name xyz -- ls -l
# Expected: opts = ["--name", "xyz"],                   cmdline = ["ls", "-l"]
#   Actual: opts = ["--name", "xyz", "--", "ls", "-l"], cmdline = []

run --name xyz --
# Expected: opts = ["--name", "xyz"],       cmdline = []
#   Actual: opts = ["--name", "xyz", "--"], cmdline = []

run --name xyz
# Expected: opts = ["--name", "xyz"], cmdline = []
#   Actual: opts = ["--name", "xyz"], cmdline = []

run -- ls -l
# Expected: opts = [], cmdline = ["ls", "-l"]
#   Actual: opts = [], cmdline = ["ls", "-l"]

run --
# Expected: opts = [], cmdline = []
#   Actual: opts = [], cmdline = []

run
# Expected: opts = [], cmdline = []
#   Actual: opts = [], cmdline = []

Describe the solution you'd like

last should take precedence over allow_hyphen_values.

Alternatives, if applicable

value_terminator = "--" would be an alternative.
However, it is ambiguous when the arguments does not contain the terminator --.

On the other hand, last requires the double hyphen -- in the arguments.
This is why I prefer last to value_terminator. last is less confusing.

See comparison below:

use clap::Parser;

// This uses `value_terminator = "--"`.
#[derive(Clone, Debug, Parser)]
pub struct ValueTerminator {
    #[arg(allow_hyphen_values = true, value_terminator = "--")]
    pub opts: Vec<String>,

    #[arg(allow_hyphen_values = true)]
    pub cmdline: Vec<String>,
}

// This uses `last = true`.
#[derive(Clone, Debug, Parser)]
pub struct Last {
    #[arg(allow_hyphen_values = true)]
    pub opts: Vec<String>,

    #[arg(allow_hyphen_values = true, last = true)]
    pub cmdline: Vec<String>,
}

fn main() {
    // `value_termiator = "--"` is ambiguous.
    // In all 3 examples below, the arguments were parsed as `opts`.
    // The seconds one will be parsed correctly once #5040 is fixed.

    let args = ValueTerminator::parse_from(["program", "--name", "xyz"]);
    println!("opts = {:?}, cmdline = {:?}", args.opts, args.cmdline);
    // opts = ["--name", "xyz"], cmdline = []

    let args = ValueTerminator::parse_from(["program", "--", "ls", "-l"]);
    println!("opts = {:?}, cmdline = {:?}", args.opts, args.cmdline);
    // opts = ["ls", "-l"], cmdline = []
    // issue: #5040 

    let args = ValueTerminator::parse_from(["program", "ls", "-l"]);
    println!("opts = {:?}, cmdline = {:?}", args.opts, args.cmdline);
    // opts = ["ls", "-l"], cmdline = []


    // `last = true` is more explicit.

    let args = Last::parse_from(["program", "--name", "xyz"]);
    println!("opts = {:?}, cmdline = {:?}", args.opts, args.cmdline);
    // Expected: opts = ["--name", "xyz"], cmdline = []
    //   Actual: opts = ["--name", "xyz"], cmdline = []

    let args = Last::parse_from(["program", "--", "ls", "-l"]);
    println!("opts = {:?}, cmdline = {:?}", args.opts, args.cmdline);
    // Expected: opts = [], cmdline = ["ls", "-l"]
    //   Actual: opts = [], cmdline = ["ls", "-l"]

    let args = Last::parse_from(["program", "ls", "-l"]);
    println!("opts = {:?}, cmdline = {:?}", args.opts, args.cmdline);
    // Expected: opts = ["ls", "-l"], cmdline = []
    //   Actual: opts = ["ls", "-l"], cmdline = []
    //
    // `["ls", "-l"]` is parsed as `opts` in this case,
    // but this is correct because `--` is not specified.
}

(EDIT: Additional notes)
The difference between last and value_terminator = "--" will be smaller after various bug fixes.
In the examples I've given so far, the difference will be limited to the following cases:

alias run='cargo run --quiet --'

run --name xyz -- ls -l
# ValueTerminator::parse();
# Expected: opts = ["--name", "xyz"], cmdline = ["ls", "-l"]
#   Actual: opts = ["--name", "xyz"], cmdline = ["ls", "-l"]
#
# Last::parse();
# Expected: opts = ["--name", "xyz"],                   cmdline = ["ls", "-l"]
#   Actual: opts = ["--name", "xyz", "--", "ls", "-l"], cmdline = []

run --name xyz --
# ValueTerminator::parse();
# Expected: opts = ["--name", "xyz"], cmdline = []
#   Actual: opts = ["--name", "xyz"], cmdline = []
#
# Last::parse();
# Expected: opts = ["--name", "xyz"],       cmdline = []
#   Actual: opts = ["--name", "xyz", "--"], cmdline = []

Additional Context

(EDIT: Deleted, since it was poorly explained and hard to understand.)

@mamekoro mamekoro added the C-enhancement Category: Raise on the bar on expectations label Jul 22, 2023
@epage epage added A-parsing Area: Parser's logic and needs it changed somehow. S-waiting-on-decision Status: Waiting on a go/no-go before implementing labels Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-enhancement Category: Raise on the bar on expectations S-waiting-on-decision Status: Waiting on a go/no-go before implementing
Projects
None yet
Development

No branches or pull requests

2 participants