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

Subcommand Conflicts with Argument (derive feature) #5513

Open
2 tasks done
lubomirkurcak opened this issue Jun 1, 2024 · 3 comments
Open
2 tasks done

Subcommand Conflicts with Argument (derive feature) #5513

lubomirkurcak opened this issue Jun 1, 2024 · 3 comments
Labels
A-parsing Area: Parser's logic and needs it changed somehow. A-validators Area: ArgMatches validation logi C-bug Category: Updating dependencies S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing

Comments

@lubomirkurcak
Copy link

Please complete the following tasks

Rust Version

rustc 1.78.0 (9b00956e5 2024-04-29)

Clap Version

clap = { version = "4.5.4", features = ["derive"] }

Minimal reproducible code

use clap::{Parser, Subcommand};

#[derive(Debug, Parser)]
struct Cli {
    name: String,
    #[command(subcommand)]
    command: Commands,
}

#[derive(Debug, Subcommand)]
enum Commands {
    Test,
}

fn main() {
    let args = Cli::parse();

    dbg!(&args);
}

Steps to reproduce the bug with the above code

OK

Positional name argument is hello, subcommand is test:

cargo run -- hello test

BAD

Positional name argument is test, subcommand is test:

cargo run -- test test

Actual Behaviour

OK

cargo run -- hello test
[src\main.rs:18:5] &args = Cli {
    name: "hello",
    command: Test,
}

BAD

cargo run -- test test
error: unexpected argument 'test' found

Usage: testing.exe <NAME> test

For more information, try '--help'.

Expected Behaviour

Clap should parse the arguments positionally:

cargo run -- test test
[src\main.rs:18:5] &args = Cli {
    name: "test",
    command: Test,
}

Additional Context

No response

Debug Output

cargo run -- test test
[clap_builder::builder::command]Command::_do_parse
[clap_builder::builder::command]Command::_build: name="testing"
[clap_builder::builder::command]Command::_propagate:testing
[clap_builder::builder::command]Command::_check_help_and_version:testing expand_help_tree=false
[clap_builder::builder::command]Command::long_help_exists
[clap_builder::builder::command]Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]Command::_check_help_and_version: Building help subcommand
[clap_builder::builder::command]Command::_propagate_global_args:testing
[clap_builder::builder::debug_asserts]Command::_debug_asserts
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:name
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]Command::_verify_positionals
[clap_builder::parser::parser]Parser::get_matches_with
[clap_builder::parser::parser]Parser::get_matches_with: Begin parsing '"test"'
[clap_builder::parser::parser]Parser::possible_subcommand: arg=Ok("test")
[clap_builder::parser::parser]Parser::get_matches_with: sc=Some("test")
[clap_builder::parser::parser]Parser::parse_subcommand
[ clap_builder::output::usage]Usage::get_required_usage_from: incls=[], matcher=false, incl_last=true
[ clap_builder::output::usage]Usage::get_required_usage_from: unrolled_reqs=["name"]
[ clap_builder::output::usage]Usage::get_required_usage_from:iter:"name" arg is_present=false
[ clap_builder::output::usage]Usage::get_required_usage_from: ret_val=[StyledStr("")]
[clap_builder::builder::command]Command::_build_subcommand Setting bin_name of test to "testing.exe test"
[clap_builder::builder::command]Command::_build_subcommand Setting display_name of test to "testing-test"
[clap_builder::builder::command]Command::_build: name="test"
[clap_builder::builder::command]Command::_propagate:test
[clap_builder::builder::command]Command::_check_help_and_version:test expand_help_tree=false
[clap_builder::builder::command]Command::long_help_exists
[clap_builder::builder::command]Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]Command::_propagate_global_args:test
[clap_builder::builder::debug_asserts]Command::_debug_asserts
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]Command::_verify_positionals
[clap_builder::parser::parser]Parser::parse_subcommand: About to parse sc=test
[clap_builder::parser::parser]Parser::get_matches_with
[clap_builder::parser::parser]Parser::get_matches_with: Begin parsing '"test"'
[clap_builder::parser::parser]Parser::possible_subcommand: arg=Ok("test")
[clap_builder::parser::parser]Parser::get_matches_with: sc=None
[clap_builder::parser::parser]Parser::get_matches_with: Positional counter...1
[clap_builder::parser::parser]Parser::get_matches_with: Low index multiples...false
[ clap_builder::output::usage]Usage::create_usage_with_title
[ clap_builder::output::usage]Usage::create_usage_no_title
[ clap_builder::output::usage]Usage::write_help_usage
[ clap_builder::output::usage]Usage::write_arg_usage; incl_reqs=true
[ clap_builder::output::usage]Usage::needs_options_tag
[ clap_builder::output::usage]Usage::needs_options_tag:iter: f=help
[ clap_builder::output::usage]Usage::needs_options_tag:iter Option is built-in
[ clap_builder::output::usage]Usage::needs_options_tag: [OPTIONS] not required
[ clap_builder::output::usage]Usage::write_args: incls=[]
[ clap_builder::output::usage]Usage::get_args: unrolled_reqs=[]
[ clap_builder::output::usage]Usage::write_subcommand_usage
[ clap_builder::output::usage]Usage::create_usage_with_title: usage=Usage: testing.exe test
[clap_builder::builder::command]Command::color: Color setting...
[clap_builder::builder::command]Auto
[clap_builder::builder::command]Command::color: Color setting...
[clap_builder::builder::command]Auto
error: unexpected argument 'test' found

Usage: testing.exe test

For more information, try '--help'.

@lubomirkurcak lubomirkurcak added the C-bug Category: Updating dependencies label Jun 1, 2024
@epage epage added A-parsing Area: Parser's logic and needs it changed somehow. A-validators Area: ArgMatches validation logi S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing labels Jun 4, 2024
@epage
Copy link
Member

epage commented Jun 4, 2024

We have subcommand_precedence_over_arg but that only applies to options; subcommands always have precedence over positionals today (the name makes it sounds more general).

Some potential routes to go

Do nothing, discouraging mixing of positionals being additive with subcommands. maybe we could add some debug asserts to help with this though that can get complicated with various settings.

We break compatibility and default to positionals having higher precedence, controlled by subcommand_precedence_over_arg.

The parser could detect there are unsatisfied requireds and ensure give them precedene. Like with the debug asserts for the "do nothing" solution, this get complicated due to the various states we can be in

Add a new setting to toggle control over this. We've generally been working to shrinking the API, rather than growing it. The more features we add to the API, the less value we get out of them because they become harder to discover and be used.

@ZakharEl
Copy link

I see no reason to do this. derive should not incur any runtime costs for unused features, only compile time. I agree with Lubomirkurcak. This makes your library unuseable for many use cases.

@ZakharEl
Copy link

I would agree with Epage's case if the arg was an optional arg though.

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. A-validators Area: ArgMatches validation logi C-bug Category: Updating dependencies S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing
Projects
None yet
Development

No branches or pull requests

3 participants