Allow literal and dynamic values for version in clap_derive #3034

gibfahn · 2021-11-16T22:26:30Z

Please complete the following tasks

I have searched the discussions
I have searched the existing issues

Rust Version

rustc 1.56.1 (59eed8a2a 2021-11-01)

Clap Version

3.0.0-beta.5

Minimal reproducible code

use clap::Parser;

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

#[derive(Parser)]
// Doesn't work unless you uncomment this line:
// #[clap(version = env!("CARGO_PKG_VERSION"))]
pub struct Opts {}

Steps to reproduce the bug with the above code

By default the version in the Cargo.toml is not picked up. The --help command doesn't show the version, and the --version command fails.

❯ cargo run -- --help
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/clap_repro --help`
clap_repro 

USAGE:
    clap_repro

OPTIONS:
    -h, --help    Print help information

❯ cargo run -- --version

Running `target/debug/clap_repro --version`
error: Found argument '--version' which wasn't expected, or isn't valid in this context

	If you tried to supply `--version` as a value rather than a flag, use `-- --version`

USAGE:
    clap_repro

For more information try --help

If you uncomment the version line below things start to work.

Actual Behaviour

Version must be manually set.

Expected Behaviour

With structopt the version is picked up automatically:

use structopt::StructOpt;

fn main() {
    Opts::from_args();
}

#[derive(StructOpt)]
pub struct Opts {}

❯ cargo run -- --help
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/clap_repro --help`
clap_repro 0.1.0

USAGE:
    clap_repro

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

❯ cargo run -- --version
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/clap_repro --version`
clap_repro 0.1.0

Additional Context

Sorry if this was an intentional change, I didn't see anything in the docs about it.

Debug Output

Debug output of --version

❯ cargo run -- --help
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/clap_repro --help`
[            clap::build::app] 	App::_do_parse
[            clap::build::app] 	App::_build
[            clap::build::app] 	App::_propagate:clap_repro
[            clap::build::app] 	App::_check_help_and_version
[            clap::build::app] 	App::_check_help_and_version: Removing generated version
[            clap::build::app] 	App::_propagate_global_args:clap_repro
[            clap::build::app] 	App::_derive_display_order:clap_repro
[clap::build::app::debug_asserts] 	App::_debug_asserts
[clap::build::arg::debug_asserts] 	Arg::_debug_asserts:help
[         clap::parse::parser] 	Parser::get_matches_with
[         clap::parse::parser] 	Parser::_build
[         clap::parse::parser] 	Parser::_verify_positionals
[         clap::parse::parser] 	Parser::get_matches_with: Begin parsing 'RawOsStr("--help")' ([45, 45, 104, 101, 108, 112])
[         clap::parse::parser] 	Parser::get_matches_with: Positional counter...1
[         clap::parse::parser] 	Parser::get_matches_with: Low index multiples...false
[         clap::parse::parser] 	Parser::possible_subcommand: arg=RawOsStr("--help")
[         clap::parse::parser] 	Parser::get_matches_with: sc=None
[         clap::parse::parser] 	Parser::parse_long_arg
[         clap::parse::parser] 	Parser::parse_long_arg: cur_idx:=1
[         clap::parse::parser] 	Parser::parse_long_arg: Does it contain '='...
[         clap::parse::parser] 	No
[         clap::parse::parser] 	Parser::parse_long_arg: Found valid opt or flag '--help'
[         clap::parse::parser] 	Parser::check_for_help_and_version_str
[         clap::parse::parser] 	Parser::check_for_help_and_version_str: Checking if --RawOsStr("help") is help or version...
[         clap::parse::parser] 	Help
[         clap::parse::parser] 	Parser::get_matches_with: After parse_long_arg HelpFlag
[         clap::parse::parser] 	[         clap::parse::parser] 	Parser::use_long_help
Parser::help_err: use_long=false
[         clap::parse::parser] 	Parser::use_long_help
[          clap::output::help] 	Help::new
[          clap::output::help] 	Help::write_help
[          clap::output::help] 	should_show_arg: use_long=false, arg=help
[          clap::output::help] 	Help::write_templated_help
[          clap::output::help] 	Help::write_before_help
[          clap::output::help] 	Help::write_bin_name
[         clap::output::usage] 	Usage::create_usage_no_title
[         clap::output::usage] 	Usage::create_help_usage; incl_reqs=true
[         clap::output::usage] 	Usage::get_required_usage_from: incls=[], matcher=false, incl_last=false
[         clap::output::usage] 	Usage::get_required_usage_from: unrolled_reqs={}
[         clap::output::usage] 	Usage::get_required_usage_from: ret_val=[]
[         clap::output::usage] 	Usage::needs_options_tag
[         clap::output::usage] 	Usage::needs_options_tag:iter: f=help
[         clap::output::usage] 	Usage::needs_options_tag:iter Option is built-in
[         clap::output::usage] 	Usage::needs_options_tag: [OPTIONS] not required
[         clap::output::usage] 	Usage::create_help_usage: usage=clap_repro
[          clap::output::help] 	Help::write_all_args
[          clap::output::help] 	should_show_arg: use_long=false, arg=help
[          clap::output::help] 	Help::write_args
[          clap::output::help] 	should_show_arg: use_long=false, arg=help
[          clap::output::help] 	Help::write_args: Current Longest...2
[          clap::output::help] 	Help::write_args: New Longest...6
[          clap::output::help] 	should_show_arg: use_long=false, arg=help
[          clap::output::help] 	Help::spec_vals: a=--help
[          clap::output::help] 	Help::spec_vals: a=--help
[          clap::output::help] 	Help::short
[          clap::output::help] 	Help::long
[          clap::output::help] 	Help::val: arg=help
[          clap::output::help] 	Help::val: Has switch...
[          clap::output::help] 	Yes
[          clap::output::help] 	Help::val: nlh...false
[          clap::output::help] 	Help::help
[          clap::output::help] 	Help::help: Next Line...false
[          clap::output::help] 	Help::help: Too long...
[          clap::output::help] 	No
[          clap::output::help] 	Help::write_after_help
[           clap::output::fmt] 	is_a_tty: stderr=false
clap_repro 

USAGE:
    clap_repro

OPTIONS:
    -h, --help    Print help information

Debug output of --version

❯ cargo run -- --version
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/clap_repro --version`
[            clap::build::app] 	App::_do_parse
[            clap::build::app] 	App::_build
[            clap::build::app] 	App::_propagate:clap_repro
[            clap::build::app] 	App::_check_help_and_version
[            clap::build::app] 	App::_check_help_and_version: Removing generated version
[            clap::build::app] 	App::_propagate_global_args:clap_repro
[            clap::build::app] 	App::_derive_display_order:clap_repro
[clap::build::app::debug_asserts] 	App::_debug_asserts
[clap::build::arg::debug_asserts] 	Arg::_debug_asserts:help
[         clap::parse::parser] 	Parser::get_matches_with
[         clap::parse::parser] 	Parser::_build
[         clap::parse::parser] 	Parser::_verify_positionals
[         clap::parse::parser] 	Parser::get_matches_with: Begin parsing 'RawOsStr("--version")' ([45, 45, 118, 101, 114, 115, 105, 111, 110])
[         clap::parse::parser] 	Parser::get_matches_with: Positional counter...1
[         clap::parse::parser] 	Parser::get_matches_with: Low index multiples...false
[         clap::parse::parser] 	Parser::possible_subcommand: arg=RawOsStr("--version")
[         clap::parse::parser] 	Parser::get_matches_with: sc=None
[         clap::parse::parser] 	Parser::parse_long_arg
[         clap::parse::parser] 	Parser::parse_long_arg: cur_idx:=1
[         clap::parse::parser] 	Parser::parse_long_arg: Does it contain '='...
[         clap::parse::parser] 	No
[         clap::parse::parser] 	Parser::possible_long_flag_subcommand: arg=RawOsStr("version")
[         clap::parse::parser] 	Parser::get_matches_with: After parse_long_arg NoMatchingArg { arg: "version" }
[         clap::parse::parser] 	Parser::did_you_mean_error: arg=version
[         clap::parse::parser] 	Parser::did_you_mean_error: longs=["help"]
[         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::get_required_usage_from: incls=[], matcher=false, incl_last=false
[         clap::output::usage] 	Usage::get_required_usage_from: unrolled_reqs={}
[         clap::output::usage] 	Usage::get_required_usage_from: ret_val=[]
[         clap::output::usage] 	Usage::needs_options_tag
[         clap::output::usage] 	Usage::needs_options_tag:iter: f=help
[         clap::output::usage] 	Usage::needs_options_tag:iter Option is built-in
[         clap::output::usage] 	Usage::needs_options_tag: [OPTIONS] not required
[         clap::output::usage] 	Usage::create_help_usage: usage=clap_repro
[           clap::output::fmt] 	is_a_tty: stderr=true
error: Found argument '--version' which wasn't expected, or isn't valid in this context

	If you tried to supply `--version` as a value rather than a flag, use `-- --version`

USAGE:
    clap_repro

For more information try --help

The text was updated successfully, but these errors were encountered:

gibfahn · 2021-11-16T22:29:06Z

Put the above into a repo in case a full repro is more useful: https://github.com/gibfahn/clap_repro

mkayaalp · 2021-11-16T22:57:15Z

I think it works with #[clap(version)] instead of #[clap(version = env!("CARGO_PKG_VERSION"))]. But isn't that supposed to be the default?

gibfahn · 2021-11-18T11:13:38Z

Ahh, you're right @mkayaalp , that does work.

But yeah, same question, isn't this supposed to be the default?

epage · 2021-11-18T14:27:48Z

In structopt, the way you opt-out of automatic version detection is with a no_version attribute.

Looks like this was changed in clap_derive in #1676 but unfortunately there isn't an explanation as to why.

My guesses

Avoid special cased opt-out attributes
Do what the user says rather than guess
Some weird behavior between (what are now called) clap::Parser and #[flatten], #[subcommand] or in defining the subcommand itself.

I am personally torn on this. Being more explicit has its benefits but requiring everyone to know a set of attributes they need to se is a pain (e.g. see the discussion on changing App defaults).

epage · 2021-11-18T14:29:18Z

Even if we keep this, we should make all example code specify version, etc like with #3028 to raise visibility

mkayaalp · 2021-11-18T19:30:47Z

I think I like the following snippet from that commit. Maybe it should be in the README.md example:

#[derive(Clap, Debug)]
#[clap(author, about, version)]
//     ^^^^^^                   <- derive author from Cargo.toml
//             ^^^^^            <- derive description from Cargo.toml
//                    ^^^^^^^   <- derive version from Cargo.toml
struct Opt {}

epage · 2021-11-18T19:39:58Z

Probably too detailed for high level examples but we should defintely talk about it where we break down all of the "magic" methods (methods derived from attributes after some post-processing by us)

mkayaalp · 2021-11-18T19:45:46Z

There is already an example with explicit version and author attributes (for top-level and subcommand). Top-level could be changed to show Cargo.toml is used by default. Current one:

#[derive(Parser)]
#[clap(version = "1.0", author = "Kevin K. <kbknapp@gmail.com>")]
struct Opts {
    //...
    #[clap(subcommand)]
    subcmd: SubCommand,
}

#[derive(Parser)]
enum SubCommand {
    #[clap(version = "1.3", author = "Someone E. <someone_else@other.com>")]
    Test(Test),
}

pksunkara · 2021-11-18T21:00:16Z

In structopt, the way you opt-out of automatic version detection is with a no_version attribute.

Looks like this was changed in clap_derive in #1676 but unfortunately there isn't an explanation as to why.

I think the discussion was a lot fragmented, some of it happening on structopt and some of it on clap.

My guesses

Avoid special cased opt-out attributes

Yup. Especially when we had author and about. no_version looks inconsistent.

Do what the user says rather than guess

Exactly this too. Even in normal API, we explicitly ask the author to specify author, about and version. We should be doing the same in derive API.

Some weird behavior between (what are now called) clap::Parser and #[flatten], #[subcommand] or in defining the subcommand itself.

Also this, Apparently it had some issues with parsing and stuff in structopt. But I am not exactly clear on what they were.

requiring everyone to know a set of attributes they need to use is a pain (e.g. see the discussion on changing App defaults).

I think this can be solved by what you said in the other comment regarding documentation.

but we should defintely talk about it where we break down all of the "magic" methods (methods derived from attributes after some post-processing by us)

I am tagging this for 3.1 milestone because we want to support literal and dynamic values for this method too. Similar to subcommand name in #2751

mkayaalp · 2021-11-18T23:31:36Z

I am tagging this for 3.1 milestone because we want to support literal and dynamic values for this method too. Similar to subcommand name in #2751

I was with you until the end there. The issue was not that #[clap(version = env!("CARGO_PKG_VERSION"))] did not work. You can use dynamic values for version unlike in #2751.

The issue was the version not being set without the version attribute. It seems there is agreement that it should be explicit. So, I would suggest closing the issue instead.

I think the "magic" about the version attribute handling is that it defaults to ${CARGO_PKG_VERSION}.

pksunkara · 2021-11-19T00:27:08Z

You are right. I misunderstood part of the issue being that dynamic values not working right now. My bad.

Folks getting started with clap are likely to want to use the automatic version and author parsing initially. Hopefully those who do want to hard-code a value will note that that can be done as well from the SubCommand version parameter. Refs: clap-rs#3034

gibfahn added the C-bug Category: Updating dependencies label Nov 16, 2021

gibfahn changed the title ~~clap_derive doesn't pick up the version from the Cargo.toml~~ clap_derive doesn't pick up the version from the Cargo.toml by default Nov 18, 2021

pksunkara added this to the 3.1 milestone Nov 18, 2021

pksunkara added 💸 $10 A-derive Area: #[derive]` macro API D: easy C-enhancement Category: Raise on the bar on expectations and removed C-bug Category: Updating dependencies labels Nov 18, 2021

pksunkara changed the title ~~clap_derive doesn't pick up the version from the Cargo.toml by default~~ Allow literal and dynamic values for version in clap_derive Nov 18, 2021

pksunkara closed this as completed Nov 19, 2021

epage mentioned this issue Nov 23, 2021

Update to abscissa 0.6.0-beta.1 and clap 3.0.0-beta.5 informalsystems/hermes#1576

Merged

5 tasks

gibfahn mentioned this issue Nov 25, 2021

docs: use automatic version and author parsing in examples #3051

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow literal and dynamic values for version in clap_derive #3034

Allow literal and dynamic values for version in clap_derive #3034

gibfahn commented Nov 16, 2021

gibfahn commented Nov 16, 2021 •

edited

mkayaalp commented Nov 16, 2021

gibfahn commented Nov 18, 2021

epage commented Nov 18, 2021

epage commented Nov 18, 2021

mkayaalp commented Nov 18, 2021

epage commented Nov 18, 2021

mkayaalp commented Nov 18, 2021

pksunkara commented Nov 18, 2021

mkayaalp commented Nov 18, 2021

pksunkara commented Nov 19, 2021

Allow literal and dynamic values for version in clap_derive #3034

Allow literal and dynamic values for version in clap_derive #3034

Comments

gibfahn commented Nov 16, 2021

Please complete the following tasks

Rust Version

Clap Version

Minimal reproducible code

Steps to reproduce the bug with the above code

Actual Behaviour

Expected Behaviour

Additional Context

Debug Output

gibfahn commented Nov 16, 2021 • edited

mkayaalp commented Nov 16, 2021

gibfahn commented Nov 18, 2021

epage commented Nov 18, 2021

epage commented Nov 18, 2021

mkayaalp commented Nov 18, 2021

epage commented Nov 18, 2021

mkayaalp commented Nov 18, 2021

pksunkara commented Nov 18, 2021

mkayaalp commented Nov 18, 2021

pksunkara commented Nov 19, 2021

gibfahn commented Nov 16, 2021 •

edited