Skip to content

refactor: Use clap to suggest alternative argument for unsupported arguments#12529

Merged
bors merged 4 commits intorust-lang:masterfrom
weihanglo:clap-suggestions
Aug 22, 2023
Merged

refactor: Use clap to suggest alternative argument for unsupported arguments#12529
bors merged 4 commits intorust-lang:masterfrom
weihanglo:clap-suggestions

Conversation

@weihanglo
Copy link
Member

@weihanglo weihanglo commented Aug 19, 2023 •
edited
Loading

What does this PR try to resolve?

Part of #11702.

This pull request replaces manual errors with clap native UnknownArgumentValueParser::suggest to give uses alternative arugments, including

  • Suggest --no-fail-fast for --keep-going in cargo test and cargo bench.
  • Suggest cargo-vendor vendor for unsupported cargo tree arguments
    • --no-merge-sources
    • --relative-path
    • --only-git-deps
    • --disallow-duplicates

This PR also renames fn arg_jobs(…) and friends to more reasonable names.

How should we test and review this PR?

cargo t --keep-going
cargo vendor --no-merge-sources
cargo vendor --relative-path
cargo vendor --only-git-deps
cargo vendor --disallow-duplicates

Before

$ cargo vendor --no-merge-sources
error: the crates.io `cargo vendor` command has now been merged into Cargo itself
and does not support the flag `--no-merge-sources` currently; to continue using the flag you
can execute `cargo-vendor vendor ...`, and if you would like to see this flag
supported in Cargo itself please feel free to file an issue at
https://github.com/rust-lang/cargo/issues/new

After

$ cargo vendor --no-merge-sources
error: unexpected argument '--no-merge-sources' found

  tip: the crates.io `cargo vendor` command has been merged into Cargo
  tip: and the flag `--no-merge-sources` isn't supported currently
  tip: to continue using the flag, execute `cargo-vendor vendor ...`
  tip: to suggest this flag supported in Cargo, file an issue at <https://github.com/rust-lang/cargo/issues/new>

Usage: cargo vendor [OPTIONS] [path]

For more information, try '--help'.

@rustbot
Copy link
Collaborator

rustbot commented Aug 19, 2023

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@weihanglo weihanglo mentioned this pull request Aug 19, 2023
Open
2 tasks
@weihanglo
Copy link
Member Author

weihanglo commented Aug 19, 2023

This is current blocked on clap-rs/clap#5081.

@epage
Copy link
Contributor

epage commented Aug 21, 2023

This is current blocked on clap-rs/clap#5081.

This can be worked around on the clap side. We could add to the prelude a maybe_flag function that calls try_get_one and filters out the type-discrepancy error.

@weihanglo weihanglo marked this pull request as ready for review August 22, 2023 06:53
@weihanglo
Copy link
Member Author

weihanglo commented Aug 22, 2023

@epage this is ready for review. Moved some functional tests to UI tests since it makes more sense matching the entire clap output there.

@weihanglo
Copy link
Member Author

weihanglo commented Aug 22, 2023

Blocked on #12539 bumping the version of cargo.

epage
epage reviewed Aug 22, 2023
View reviewed changes
epage
epage reviewed Aug 22, 2023
View reviewed changes
@epage epage changed the title refactor: suggest alternative argument for well-known unknown argument fix: suggest alternative argument for well-known unknown argument Aug 22, 2023
@epage
Copy link
Contributor

epage commented Aug 22, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Aug 22, 2023

📌 Commit a31aaac has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 22, 2023
@epage epage changed the title fix: suggest alternative argument for well-known unknown argument refactor: suggest alternative argument for well-known unknown argument Aug 22, 2023
@epage epage changed the title refactor: suggest alternative argument for well-known unknown argument refactor: Use clap to suggest alternative argument for unsupported arguments Aug 22, 2023
@bors
Copy link
Contributor

bors commented Aug 22, 2023

⌛ Testing commit a31aaac with merge a4c5f11...

bors added a commit that referenced this pull request Aug 22, 2023
@bors
Auto merge of #12529 - weihanglo:clap-suggestions, r=epage
a4c5f11 
refactor: Use clap to suggest alternative argument for unsupported arguments
@bors
Copy link
Contributor

bors commented Aug 22, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 22, 2023
@weihanglo
test: move --keep-going suggestion match tests to UI tests
77da514 
We don't want to overly match to suggestion help from clap.
@epage
Copy link
Contributor

epage commented Aug 22, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Aug 22, 2023

📌 Commit 77da514 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 22, 2023
@bors
Copy link
Contributor

bors commented Aug 22, 2023

⌛ Testing commit 77da514 with merge 50ba4bf...

@bors
Copy link
Contributor

bors commented Aug 22, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 50ba4bf to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Aug 22, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 50ba4bf to master...

@bors
Copy link
Contributor

bors commented Aug 22, 2023

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

@bors bors merged commit 50ba4bf into rust-lang:master Aug 22, 2023
@weihanglo weihanglo deleted the clap-suggestions branch August 22, 2023 20:06
@epage epage mentioned this pull request Aug 23, 2023
Closed
10 tasks
@weihanglo weihanglo mentioned this pull request Aug 23, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 23, 2023
@bors
Auto merge of rust-lang#115121 - weihanglo:update-cargo, r=weihanglo
e3a0e38 
Update cargo

3 commits in 80eca0e58fb2ff52c1e94fc191b55b37ed73e0e4..2cc50bc0b63ad20da193e002ba11d391af0104b7
2023-08-19 00:52:06 +0000 to 2023-08-22 22:43:08 +0000
- config: merge lists in precedence order (rust-lang/cargo#12515)
- ci: test `resolver-tests` in a separate job (rust-lang/cargo#12540)
- refactor: Use clap to suggest alternative argument for unsupported arguments (rust-lang/cargo#12529)

r? ghost
@ehuss ehuss added this to the 1.74.0 milestone Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@epage epage epage left review comments

Assignees

@epage epage

Labels

A-cli Area: Command-line interface, option parsing, etc. Command-bench Command-build Command-check Command-doc Command-fix Command-install Command-package Command-publish Command-run Command-rustc Command-rustdoc Command-test Command-vendor S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.74.0

Development

Successfully merging this pull request may close these issues.

5 participants

@weihanglo @rustbot @epage @bors @ehuss