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

Fix issue where cargo fmt --version would not display version info #5396

Merged
merged 1 commit into from Jun 22, 2022

Conversation

ytmimi
Copy link
Contributor

@ytmimi ytmimi commented Jun 21, 2022

Fixes #5395

In PR #5239 we switched from using structopt to clap. It seems that the default behavior for clap is to override the --version flag, which prevented our custom version display code from running.

The fix as outlined in clap-rs/clap#3405 was to set
#[clap(global_setting(AppSettings::NoAutoVersion))] to prevent clap from setting its own default behavior for the --version flag.

Fixes 5395

In PR 5239 we switched from using `structopt` to `clap`. It seems that
the default behavior for `clap` is to override the `--version` flag,
which prevented our custom version display code from running.

The fix as outlined in clap-rs/clap#3405 was
to set `#[clap(global_setting(AppSettings::NoAutoVersion))]` to prevent
clap from setting its own default behavior for the `--version` flag.
@ytmimi
Copy link
Contributor Author

ytmimi commented Jun 21, 2022

Note: We have a test that likely would have caught this issue but we're currently skipping it because of issues with some of our tests when they're run in rust-lang/rust

#[ignore]
#[test]
fn version() {
assert_that!(&["--version"], starts_with("rustfmt "));
assert_that!(&["--version"], starts_with("rustfmt "));
assert_that!(&["--", "-V"], starts_with("rustfmt "));
assert_that!(&["--", "--version"], starts_with("rustfmt "));
}

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks so much for the quick fix!

@calebcartwright calebcartwright added release-notes Needs an associated changelog entry and removed pr-not-reviewed labels Jun 22, 2022
@calebcartwright calebcartwright merged commit 08105e8 into rust-lang:master Jun 22, 2022
@calebcartwright calebcartwright removed the release-notes Needs an associated changelog entry label Jun 23, 2022
@ytmimi ytmimi deleted the issue_5395 branch August 7, 2022 15:19
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.

cargo-fmt version not displaying correctly
2 participants