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(cli): Make --help easier to browse #11905

Merged
merged 1 commit into from Aug 8, 2023
Merged

Conversation

epage
Copy link
Contributor

@epage epage commented Mar 28, 2023

This mirrors some of the categories from cargo help (the man pages) using clap::Arg::help_heading.
There are fewer categories to avoid extra vertical space. Instead, they
are left int the Options category but put first.

The goal is to try to make it easier to scan --help output as the list of flags can get quite long and its easy to miss what features are there.

@rustbot
Copy link
Collaborator

rustbot commented Mar 28, 2023

r? @weihanglo

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

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2023
epage added a commit to epage/cargo that referenced this pull request Mar 29, 2023
This makes it easier to evaluate the usability of PRs, like rust-lang#11905
epage added a commit to epage/cargo that referenced this pull request Mar 30, 2023
This makes it easier to evaluate the usability of PRs, like rust-lang#11905
epage added a commit to epage/cargo that referenced this pull request Mar 30, 2023
This makes it easier to evaluate the usability of PRs, like rust-lang#11905
epage added a commit to epage/cargo that referenced this pull request Mar 31, 2023
This makes it easier to evaluate the usability of PRs, like rust-lang#11905
@bors
Copy link
Collaborator

bors commented May 20, 2023

☔ The latest upstream changes (presumably #12157) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

This seems awesome! I am happy to help move it forward.

At least two things need to figure out I guess:

@epage
Copy link
Contributor Author

epage commented May 22, 2023

Will it break shell completions?

People better not be relying on --help for compeltions; clap has already changed things out from under people

Should we somehow first integrate #11912?

That is why I have not been working to move this forward.

epage added a commit to epage/cargo that referenced this pull request Jun 6, 2023
This makes it easier to evaluate the usability of PRs, like rust-lang#11905
epage added a commit to epage/cargo that referenced this pull request Jul 24, 2023
This makes it easier to evaluate the usability of PRs, like rust-lang#11905
bors added a commit that referenced this pull request Jul 24, 2023
test(cli): Track --help output

### What does this PR try to resolve?

This makes it easier to evaluate the usability of PRs, like #11905

This follows the pattern of `cargo add` and `cargo remove` of putting these ui tests in `cargo_<cmd>/` directories.  `init` didn't follow this pattern, so it was renamed to `cargo_init/`.  `cargo_config.rs` was going to conflict with this, it was merged in.

We can evaluate other `<cmd>.rs` files at a later point and consolidate.

### How should we test and review this PR?

The main risks are
- Are all files linked together (`main.rs` -> `<cmd>/mod.rs` -> `<cmd>/<help>.rs`
- Are all `help/mod.rs`s pointing to the right command
@epage epage force-pushed the help_heading branch 2 times, most recently from 830931e to d500051 Compare July 25, 2023 01:05
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Thanks a lot. Let's move on!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 8, 2023

📌 Commit 853102f has been approved by weihanglo

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 8, 2023
@bors
Copy link
Collaborator

bors commented Aug 8, 2023

⌛ Testing commit 853102f with merge 2b3554f...

@bors
Copy link
Collaborator

bors commented Aug 8, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 2b3554f to master...

1 similar comment
@bors
Copy link
Collaborator

bors commented Aug 8, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 2b3554f to master...

@bors
Copy link
Collaborator

bors commented Aug 8, 2023

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

@bors bors merged commit 2b3554f into rust-lang:master Aug 8, 2023
18 of 19 checks passed
@epage epage deleted the help_heading branch August 8, 2023 21:22
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 13, 2023
Update cargo

21 commits in d78bbf4bde3c6b95caca7512f537c6f9721426ff..7e9de3f4ec3708f500bec142317895b96131e47c
2023-08-03 12:58:25 +0000 to 2023-08-13 00:47:32 +0000
- feat: remove `--keep-going` from `cargo test/bench` (rust-lang/cargo#12478)
- chore: window-sys should be a platform-specific dependency (rust-lang/cargo#12483)
- docs: make the env var source of rerun-if-env-changed clearer (rust-lang/cargo#12482)
- doc: note the backward compatible `.cargo/credential` file exists (rust-lang/cargo#12479)
- Fix elided lifetime in associated const (rust-lang/cargo#12475)
- prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal. (rust-lang/cargo#12463)
- cargo-credential: reset stdin & stdout to the Console (rust-lang/cargo#12469)
- Fix cargo remove incorrectly removing used patches (rust-lang/cargo#12454)
- chore(gh): Expand update window (rust-lang/cargo#12466)
- Fix panic when enabling http.debug for certain strings (rust-lang/cargo#12468)
- fix(cli): Make `--help` easier to browse (rust-lang/cargo#11905)
- fix: preserve jobserver file descriptors on rustc invocation to get `TargetInfo` (rust-lang/cargo#12447)
- refactor: migrate to `tracing` (rust-lang/cargo#12458)
- docs: add example for cargo-credential (rust-lang/cargo#12461)
- Bail out an error when using cargo:: in custom build script (rust-lang/cargo#12332)
- Fix printing multiple warning messages for unused fields in [registries] table (rust-lang/cargo#12439)
- Update windows dependencies (rust-lang/cargo#12453)
- Rustfmt a let-else statement (rust-lang/cargo#12451)
- Add allow(internal_features) (rust-lang/cargo#12450)
- Update pretty_env_logger to 0.5 (rust-lang/cargo#12445)
- Remove build metadata from libgit2-sys dependency (rust-lang/cargo#12444)

r? `@ghost`
@ehuss ehuss added this to the 1.73.0 milestone Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment