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

Show the description of extensions via --cargo-short-description #9045

Closed
wants to merge 1 commit into from

Conversation

kpp
Copy link
Contributor

@kpp kpp commented Jan 5, 2021

I played with cargo flags and discovered there is a --list of installed extensions. However there is no description of these extensions, only names and paths (with --verbose). I added a possibility for extensions to tell cargo their description calling them with --cargo-short-description flag. The flag is questionable, but the --help command is not unified and the first line from --help almost never prints the description.

Before:

$ cargo list
    cache                
    clippy               
    deb                  
    expand               
    fmt                  
    generate-rpm         
    miri                 
    nono                 

After:

    cache                
    clippy               
    deb                  Make Debian packages (.deb) easily with a Cargo subcommand
    expand               
    fmt                  
    generate-rpm         
    miri                 
    nono                 

I patched cargo-deb locally to make it work:

diff --git a/src/main.rs b/src/main.rs
index 4296600..83aeb59 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -39,6 +39,7 @@ fn main() {
     cli_opts.optflag("v", "verbose", "Print progress");
     cli_opts.optflag("h", "help", "Print this help menu");
     cli_opts.optflag("", "version", "Show the version of cargo-deb");
+    cli_opts.optflag("", "cargo-short-description", "Show the short description as a cargo extension");
     cli_opts.optopt("", "deb-version", "Alternate version string for package", "version");
 
     let matches = match cli_opts.parse(&args[1..]) {
@@ -52,6 +53,11 @@ fn main() {
         return;
     }
 
+    if matches.opt_present("cargo-short-description") {
+        println!("Make Debian packages (.deb) easily with a Cargo subcommand");
+        return;
+    }
+
     if matches.opt_present("version") {
         println!("{}", env!("CARGO_PKG_VERSION"));
         return;

Drawbacks:

If the flag is not supported - extensions print error to stderr except for cargo-rpm:

rpm                  error: unrecognized option `--cargo-short-description`

@rust-highfive
Copy link

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 5, 2021
@alexcrichton
Copy link
Member

Thanks for the PR!

I'm a bit hesitant about this for two reasons though. The first is that this means cargo help is invoking quite a few processes just to get help text, and with more than a handful of cargo subcommands that could actually take a noticeable amouont of time. The second is that currently we haven't documented any sort of interface between Cargo and custom subcommands (other than the one way we forward arguments today), so it's unlikely that any existing subcommand would work with this new protocol (and it may even produce very surprising results in some cases).

I think it might be better for custom subcommands to somehow "opt-in" to having this sort of short help text, but unfortunately I don't really know how that would best be done myself. Maybe with some manifest metadata Cargo scrapes on installation?

@kpp
Copy link
Contributor Author

kpp commented Jan 6, 2021

The first is that this means cargo help is invoking quite a few processes just to get help text

Not on cargo help but on cargo --list which is IMO far less frequent to be called by a user. Still getting this info requires some amount of time, that's why I invoke processes in that function instead of list_commands + extending CommandInfo::External with about. At first I tried this approach but it really took a significant amount of time to call extensions.

Then I tried another approach calling extensions while printing the info, which reduced the latency, and it worked. It prints the list of subcommands almost immediately. Also this functionality can be hidden behind --verbose flag.

it's unlikely that any existing subcommand would work with this new protocol (and it may even produce very surprising results in some cases

There is a list of known third-party subcommands, we can embed short descriptions right into cargo(like you already do for builtins) which will work extremely fast. Or you can ask me me ping every repo with cargo subcommand to file an issue to support this feature =)

Maybe with some manifest metadata Cargo scrapes on installation?

It will require a large amount of work to actually figure out how to make everybody happy. I created this PR to start a discussion whether you are interested in this feature at all which is achievable in 10 lines of code.

@alexcrichton
Copy link
Member

I agree this is easy as-is, but I disagree that this is viable as-is. This is a breaking change for existing subcommands who might not expect to be invoked with this argument. Additionally using this as you've seen can take quite some time since processes aren't exactly speedy. I think this is a fine problem to solve, but I do not believe this is the solution.

The fact that this solution fits in 10 lines is irrelevant to me, we'll want to look for a solution that both works and satisfies the constraints of maintainability in Cargo.

@kpp
Copy link
Contributor Author

kpp commented Jan 11, 2021

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants