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

Custom help option always requires an argument #4367

Open
2 tasks done
wfxr opened this issue Oct 11, 2022 · 6 comments
Open
2 tasks done

Custom help option always requires an argument #4367

wfxr opened this issue Oct 11, 2022 · 6 comments
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Milestone

Comments

@wfxr
Copy link

wfxr commented Oct 11, 2022

Please complete the following tasks

Rust Version

rustc 1.66.0-nightly (a6b7274a4 2022-10-10)

Clap Version

4.0.12

Minimal reproducible code

use clap::{ArgAction, Parser};

#[derive(Parser)]
#[command(about, version)]
#[command(disable_help_flag = true)]
pub struct App {
    #[arg(short = 'h', long, default_value = "localhost")]
    pub host: String,

    #[arg(long, action = ArgAction::Help)]
    pub help: bool,
}

fn main() {
    App::parse();
}

Steps to reproduce the bug with the above code

cargo run

Actual Behaviour

The following message was printed:

error: The following required argument was not provided: help

Usage: clap-demo [OPTIONS]

Expected Behaviour

No error printed.

Additional Context

No response

Debug Output

[      clap::builder::command]  Command::_do_parse
[      clap::builder::command]  Command::_build: name="clap-demo"
[      clap::builder::command]  Command::_propagate:clap-demo
[      clap::builder::command]  Command::_check_help_and_version:clap-demo expand_help_tree=false
[      clap::builder::command]  Command::long_help_exists
[      clap::builder::command]  Command::_check_help_and_version: Building default --version
[      clap::builder::command]  Command::_propagate_global_args:clap-demo
[clap::builder::debug_asserts]  Command::_debug_asserts
[clap::builder::debug_asserts]  Arg::_debug_asserts:host
[clap::builder::debug_asserts]  Arg::_debug_asserts:help
[clap::builder::debug_asserts]  Arg::_debug_asserts:version
[clap::builder::debug_asserts]  Command::_verify_positionals
[        clap::parser::parser]  Parser::get_matches_with
[        clap::parser::parser]  Parser::add_defaults
[        clap::parser::parser]  Parser::add_defaults:iter:host:
[        clap::parser::parser]  Parser::add_default_value: doesn't have conditional defaults
[        clap::parser::parser]  Parser::add_default_value:iter:host: has default vals
[        clap::parser::parser]  Parser::add_default_value:iter:host: wasn't used
[        clap::parser::parser]  Parser::react action=Set, identifier=None, source=DefaultValue
[   clap::parser::arg_matcher]  ArgMatcher::start_custom_arg: id="host", source=DefaultValue
[      clap::builder::command]  Command::groups_for_arg: id="host"
[   clap::parser::arg_matcher]  ArgMatcher::start_custom_arg: id="App", source=DefaultValue
[        clap::parser::parser]  Parser::push_arg_values: ["localhost"]
[        clap::parser::parser]  Parser::add_single_val_to_arg: cur_idx:=1
[      clap::builder::command]  Command::groups_for_arg: id="host"
[   clap::parser::arg_matcher]  ArgMatcher::needs_more_vals: o=host, pending=0
[   clap::parser::arg_matcher]  ArgMatcher::needs_more_vals: expected=1, actual=0
[        clap::parser::parser]  Parser::react not enough values passed in, leaving it to the validator to complain
[        clap::parser::parser]  Parser::add_defaults:iter:help:
[        clap::parser::parser]  Parser::add_default_value: doesn't have conditional defaults
[        clap::parser::parser]  Parser::add_default_value:iter:help: doesn't have default vals
[        clap::parser::parser]  Parser::add_defaults:iter:version:
[        clap::parser::parser]  Parser::add_default_value: doesn't have conditional defaults
[        clap::parser::parser]  Parser::add_default_value:iter:version: doesn't have default vals
[     clap::parser::validator]  Validator::validate
[     clap::parser::validator]  Validator::validate_conflicts
[     clap::parser::validator]  Validator::validate_exclusive
[     clap::parser::validator]  Validator::validate_required: required=ChildGraph([])
[     clap::parser::validator]  Validator::gather_requires
[     clap::parser::validator]  Validator::validate_required: is_exclusive_present=false
[   clap::parser::arg_matcher]  ArgMatcher::get_global_values: global_arg_vec=[]
[      clap::builder::command]  Command::_build: name="clap-demo"
[      clap::builder::command]  Command::_propagate:clap-demo
[      clap::builder::command]  Command::_check_help_and_version:clap-demo expand_help_tree=false
[      clap::builder::command]  Command::long_help_exists
[      clap::builder::command]  Command::_check_help_and_version: Building default --version
[      clap::builder::command]  Command::_propagate_global_args:clap-demo
[clap::builder::debug_asserts]  Command::_debug_asserts
[clap::builder::debug_asserts]  Arg::_debug_asserts:host
[clap::builder::debug_asserts]  Arg::_debug_asserts:help
[clap::builder::debug_asserts]  Arg::_debug_asserts:version
[clap::builder::debug_asserts]  Command::_verify_positionals
[      clap::builder::command]  Command::_build: name="clap-demo"
[      clap::builder::command]  Command::_build: already built
[         clap::output::usage]  Usage::create_usage_with_title
[         clap::output::usage]  Usage::create_usage_no_title
[         clap::output::usage]  Usage::create_help_usage; incl_reqs=true
[         clap::output::usage]  Usage::needs_options_tag
[         clap::output::usage]  Usage::needs_options_tag:iter: f=host
[      clap::builder::command]  Command::groups_for_arg: id="host"
[         clap::output::usage]  Usage::needs_options_tag:iter:iter: grp_s="App"
[         clap::output::usage]  Usage::needs_options_tag:iter: [OPTIONS] required
[         clap::output::usage]  Usage::get_args: incls=[]
[         clap::output::usage]  Usage::get_args: unrolled_reqs=[]
[         clap::output::usage]  Usage::get_args: ret_val=[]
[         clap::output::usage]  Usage::create_help_usage: usage=clap-demo [OPTIONS]
[      clap::builder::command]  Command::color: Color setting...
[      clap::builder::command]  Auto
[      clap::builder::command]  Command::color: Color setting...
[      clap::builder::command]  Auto
error: The following required argument was not provided: help

Usage: clap-demo [OPTIONS]
@wfxr wfxr added the C-bug Category: Updating dependencies label Oct 11, 2022
@epage
Copy link
Member

epage commented Oct 11, 2022

A workaround:

#!/usr/bin/env -S rust-script

//! ```cargo
//! [dependencies]
//! clap = { version = "=4.0.12", features = ["derive"] }
//! ```

use clap::{ArgAction, Parser};

#[derive(Parser)]
#[command(about, version)]
#[command(disable_help_flag = true)]
pub struct App {
    #[arg(short = 'h', long, default_value = "localhost")]
    pub host: String,

    #[arg(long, action = ArgAction::Help, default_value = "false")]
    pub help: bool,
}

fn main() {
    App::parse();
}

@epage
Copy link
Member

epage commented Oct 11, 2022

We probably should set the default value_parser, value, and missing value for this like other flags but I suspect that would be a breaking change.

I have also been considering adding support for () as a type to mean" don't bother reading it" which would be more appropriate.

@epage epage added M-breaking-change Meta: Implementing or merging this will introduce a breaking change. E-easy Call for participation: Experience needed to fix: Easy / not much A-derive Area: #[derive]` macro API labels Oct 11, 2022
@epage
Copy link
Member

epage commented Oct 11, 2022

Except that workaround doesn't work because we take any value to mean to show help./

Bleh. The joy of not realizing a test doesn't cover all cases

epage added a commit to epage/clap that referenced this issue Oct 11, 2022
When overriding other fields, help or version flag, globals, etc, a user
might not care about the value, so let's ignore the lookup.

Been talking about this for a while but Issue clap-rs#4367 moved this forward
because there wasn't a good way to handle this without changing
behavior.
epage added a commit to epage/clap that referenced this issue Oct 11, 2022
When overriding other fields, help or version flag, globals, etc, a user
might not care about the value, so let's ignore the lookup.

Been talking about this for a while but Issue clap-rs#4367 moved this forward
because there wasn't a good way to handle this without changing
behavior.
@epage
Copy link
Member

epage commented Oct 11, 2022

#4371 will allow

#[test]
fn custom_help_flag() {
    #[derive(Debug, Clone, Parser)]
    #[command(disable_help_flag = true)]
    struct CliOptions {
        #[arg(short = 'h', long = "verbose-help", action = ArgAction::Help, value_parser = clap::value_parser!(bool))]
        help: (),
    }

    let result = CliOptions::try_parse_from(["cmd", "--verbose-help"]);
    let err = result.unwrap_err();
    assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp);

    CliOptions::try_parse_from(["cmd"]).unwrap();
}

@wfxr
Copy link
Author

wfxr commented Oct 12, 2022

@epage Great! #4371 solves the problem perfectly. Thanks you!

@wfxr wfxr closed this as completed Oct 12, 2022
@epage
Copy link
Member

epage commented Oct 12, 2022

Going to re-open as I feel like we should add a default value parser and value for Help and Version actions so they can be used with less ceremony

@epage epage reopened this Oct 12, 2022
@epage epage added this to the 5.0 milestone Oct 12, 2022
wfxr added a commit to hackathon-2022-ticli/ticli that referenced this issue Oct 21, 2022
clap-rs/clap#4367
clap-rs/clap#4371

Signed-off-by: Wenxuan Zhang <wenxuangm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Projects
None yet
Development

No branches or pull requests

2 participants