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.

Sign up for GitHub

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

Jump to bottom

Display alias target on 'cargo help <alias>` #10193

Merged
merged 6 commits into from
Dec 14, 2021
Merged

Display alias target on 'cargo help <alias>` #10193

merged 6 commits into from
Dec 14, 2021

Conversation

sstangl
Copy link
Contributor

@sstangl sstangl commented Dec 11, 2021 

Previously, `cargo help <alias>` resolved the alias and displayed the
help for the targeted subcommand. For example, if `br` were aliased to
`build --release`, `cargo help br` would display the manpage for
cargo-build.

With this patch, it will print "'br' is aliased to 'build --release'".

Addresses issue #10138.

This is my first patch to Cargo. I attempted to follow the style of the surrounding code. Please let me know if any changes are required: happy to make them. In particular, I wasn't sure if any tests exist for this path.

@sstangl
Display alias target on 'cargo help <alias>`. Closes rust-lang#10138
7084570 
Previously, `cargo help <alias>` resolved the alias and displayed the
help for the targeted subcommand. For example, if `br` were aliased to
`build --release`, `cargo help br` would display the manpage for
cargo-build.

With this patch, it will print "'br' is aliased to 'build --release'".
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2021
@sstangl
Copy link
Contributor Author

sstangl commented Dec 11, 2021

Aha, found the test location! I'll fix that up.

@sstangl
Copy link
Contributor Author

sstangl commented Dec 11, 2021

One good UX option here may be to show the manpage if the alias is a simple command, e.g., b = build, but show the alias if it's a more complicated command, e.g., br = build --release. That would perhaps be the least surprising change, while still servicing the needs of users who define more complicated aliases. I'll wait for feedback on what direction you'd like to take.

weihanglo
weihanglo reviewed Dec 12, 2021
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great!
Might need to update doc of handle_embedded_help as well? Since we don't display man for alias anymore

cargo/src/bin/cargo/commands/help.rs

Lines 18 to 20 in 6d75259

/// Returns `true` if a man page was displayed. In this case, Cargo should
/// exit.
pub fn handle_embedded_help(config: &Config) -> bool {

src/bin/cargo/commands/help.rs Show resolved Hide resolved
src/bin/cargo/commands/help.rs Outdated Show resolved Hide resolved
src/bin/cargo/commands/help.rs Outdated Show resolved Hide resolved
sstangl and others added 4 commits December 11, 2021 20:55
@sstangl @weihanglo
Update src/bin/cargo/commands/help.rs
dc0bc58 
Co-authored-by: Weihang Lo <weihanglo@users.noreply.github.com>
@sstangl @weihanglo
Simplify check_alias() using flatten()
8a1af70 
Co-authored-by: Weihang Lo <weihanglo@users.noreply.github.com>
@sstangl
Update behavior to pass-through simple aliases
6df4b1e 
This changes the behavior so that simple aliases that directly alias a
subcommand (with no arguments) pass-through to that subcommand, while
complex aliases (with arguments) show the alias.

So for example, `cargo help b` will show the manpage for `cargo-build`,
while `cargo help my-alias`, aliased to `build --release`, will show
"`my-alias` is aliased to `build --release`".
@sstangl
fix panic if an alias is defined to ""
4c66d18
@alexcrichton
Copy link
Member

@bors: r+

Seems reasonable to me, thanks!

@bors
Copy link
Contributor

bors commented Dec 14, 2021

📌 Commit 4c66d18 has been approved by alexcrichton

@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 Dec 14, 2021
@bors
Copy link
Contributor

bors commented Dec 14, 2021

⌛ Testing commit 4c66d18 with merge c689f55...

@bors
Copy link
Contributor

bors commented Dec 14, 2021

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing c689f55 to master...

@bors bors merged commit c689f55 into rust-lang:master Dec 14, 2021
@poliorcetics
Copy link
Contributor

Thank you @sstangl !

@sstangl sstangl deleted the help-alias-10138 branch December 14, 2021 19:34
@ehuss ehuss mentioned this pull request Dec 14, 2021
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 15, 2021
@matthiaskrgr
Rollup merge of rust-lang#91940 - ehuss:update-cargo, r=ehuss
22fc403 
Update cargo

14 commits in 40dc281755137ee804bc9b3b08e782773b726e44..a359ce16073401f28b84840da85b268aa3d37c88
2021-12-06 21:54:44 +0000 to 2021-12-14 18:40:22 +0000
- Support `term.quiet` configuration (rust-lang/cargo#10152)
- Display alias target on 'cargo help &lt;alias&gt;` (rust-lang/cargo#10193)
- delete --host command and message (rust-lang/cargo#10145)
- Improve I/O error message for fingerprint of build script (rust-lang/cargo#10191)
- Explicitly mark aliases in `cargo list`. (rust-lang/cargo#10177)
- Don't emit "executable" JSON field for non-executables. (rust-lang/cargo#10171)
- Move scrape-examples docs to correct section. (rust-lang/cargo#10166)
- Do not suggest source config if nothing to vendor (rust-lang/cargo#10161)
- Bump versions of local deps. (rust-lang/cargo#10155)
- Bump to 0.60.0, update changelog (rust-lang/cargo#10154)
- Fix some profile documentation. (rust-lang/cargo#10153)
- Document lib before bin. (rust-lang/cargo#10172)
- Sync cargo-the-cli version with rustc. (rust-lang/cargo#10178)
- Remove `-Z future-incompat-report` from message displayed to user (rust-lang/cargo#10185)
@ehuss ehuss added this to the 1.59.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo left review comments

Assignees

@alexcrichton alexcrichton

Labels
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.59.0
Development

Successfully merging this pull request may close these issues.
7 participants
@sstangl @rust-highfive @alexcrichton @bors @poliorcetics @weihanglo @ehuss