rust-secure-code / cargo-supply-chain

Gather author, contributor and publisher data on crates in your dependency graph.
Apache License 2.0
315 stars 18 forks source link

Better CLI help formatting with em dashes to simplify #35

Closed Owez closed 3 years ago

Owez commented 3 years ago

Simplified the projection of the CLI help message from:

Usage: cargo supply-chain COMMAND [OPTIONS...] [-- CARGO_METADATA_OPTIONS...]

  Commands:
    authors             List all authors in the dependency graph (as specified in Cargo.toml)
    publishers          List all crates.io publishers in the dependency graph
    crates              List all crates in dependency graph and crates.io publishers for each
    update              Download the latest daily dump from crates.io to speed up other commands
  Arguments:
    --cache-max-age             The cache will be considered valid while younger than specified.
                        The format is a human recognizable duration such as `1w` or `1d 6h`.

  Any arguments after -- will be passed to `cargo metadata`, for example:
    cargo supply-chain crates -- --filter-platform=x86_64-unknown-linux-gnu

To the following:

Usage: cargo supply-chain COMMAND [OPTIONS...] [-- CARGO_METADATA_OPTIONS...]

Commands:
  authors — List all authors in the dependency graph (as specified in Cargo.toml)
  publishers — List all crates.io publishers in the dependency graph
  crates — List all crates in dependency graph and crates.io publishers for each
  update — Download the latest daily dump from crates.io to speed up other commands
  help — Displays this help message and exits

Arguments:
  --cache-max-age — The cache will be considered valid while younger than specified.
                    The format is a human readable duration such as `1w` or `1d 6h`.

Any arguments after the `--` will be passed to `cargo metadata`, for example:
  cargo supply-chain crates -- --filter-platform=x86_64-unknown-linux-gnu
Owez commented 3 years ago

Will also add a dedicated help (sub)command with help and move CLI help to a const

Shnatsel commented 3 years ago

I'm not convinced that the presentation with dashes is more readable. They look about the same to me. I'm also a bit wary about adding non-ASCII characters, but for the help text this is probably fine.

I've checked kubectl, a tool with tons of subcommands, and it uses the same formatting approach as used currently but with a bit less space between the subcommand and the explanation. Perhaps a single \ instead of \t\t, or \t followed by space would do?

help subcommand looks superfluous to me as-is (this info is usually surfaced via --help or -h). However, it could be useful to have it print more detailed explanation of what every command does. So you could run e.g. cargo supply-chain help authors and get an explanation of where this info comes from, what it means, etc. This is also where we could put details like "works offline". Thoughts?

Owez commented 3 years ago

Yeah, I can revert to ascii-only text but modern day systems which would be using compiling rust and using this tool shouldn't have an issue with ASCII.

For github and my "hack" font there's a decent size difference between the two in terms of formatting: — vs - (though not as distinct in monospace). Unless ASCII is a issue, it helps distinguish from -a - example in the future with short args. I prefer dashes over tabs in the terminal due to being known to always be 3 chars so you can compact into 80 chars all the time, but tabs can always be used of course.

It's typical for layouts to be tabbed with two spaces, as seen with clap and click in python (the two de-facto references for rust and clis in general). Yeah help can be --help, should I change it to an arg?

The main issue with pico-args is the lack of help generation, specific help for each arg and sub-command can be added but it takes a decent amount of time to write an explanation on each as it's basically a manpage in the form of a CLI.

Shnatsel commented 3 years ago

I'm mostly thinking Windows Console, which IIRC has a weird encoding. But yes, it's not a particuarly strong argument, especially for the help text.

I'd prefer to follow the established convention for help text if possible. I agree \t\t is kinda ugly in our case, perhaps \t or some such would yield better results.

Yeah help can be --help, should I change it to an arg?

Please do. I believe the base error message should be surfaced with -h and --help - that convention is pretty ironclad.

For subcommands, I slightly prefer cargo supply-chain help crates over cargo supply-chain crates --help, but that doesn't have to be in scope of this PR.

The main issue with pico-args is the lack of help generation, specific help for each arg and sub-command can be added but it takes a decent amount of time to write an explanation on each as it's basically a manpage in the form of a CLI.

I don't think pico-args help generation this would significantly change the amount of work we have to do here. We need to write all that informative text anyway.

Shnatsel commented 3 years ago

I've created #37 to deal with extended help for each subcommand, and to avoid holding up this PR.

Owez commented 3 years ago

I'm mostly thinking Windows Console, which IIRC has a weird encoding. But yes, it's not a particuarly strong argument, especially for the help text.

I'd prefer to follow the established convention for help text if possible. I agree \t\t is kinda ugly in our case, perhaps \t or some such would yield better results.

Yeah help can be --help, should I change it to an arg?

Please do. I believe the base error message should be surfaced with -h and --help - that convention is pretty ironclad.

For subcommands, I slightly prefer cargo supply-chain help crates over cargo supply-chain crates --help, but that doesn't have to be in scope of this PR.

The main issue with pico-args is the lack of help generation, specific help for each arg and sub-command can be added but it takes a decent amount of time to write an explanation on each as it's basically a manpage in the form of a CLI.

I don't think pico-args help generation this would significantly change the amount of work we have to do here. We need to write all that informative text anyway.

Alright, ill change it tomorrow :) I could support both help and --help/-h so people's instincts going to help pages can still be used whilst supporting the help which imo looks nicer also

Shnatsel commented 3 years ago

I want to release the tool soon, so I'll merge this as-is and then apply the requested changes myself. Thanks for the PR!

Owez commented 3 years ago

Oh shoot, sorry @Shnatsel ! I forgot to apply them, my bad 😔